| Message ID | 20230904161516.66751-3-vignesh.raman@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm: ci: fixes | expand |
On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: > > Force db410c to host mode to fix network issue which results in failure > to mount root fs via NFS. > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > Use fdtoverlay command to merge base device tree with an overlay > which contains the fix for USB controllers to work in host mode. > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > --- > > v2: > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > --- > drivers/gpu/drm/ci/build.sh | 5 +++++ > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > 2 files changed, 18 insertions(+) > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > index 7b014287a041..92ffd98cd09e 100644 > --- a/drivers/gpu/drm/ci/build.sh > +++ b/drivers/gpu/drm/ci/build.sh > @@ -92,6 +92,11 @@ done > > if [[ -n ${DEVICE_TREES} ]]; then > make dtbs > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > + fi > cp ${DEVICE_TREES} /lava-files/. > fi > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > new file mode 100644 > index 000000000000..57b7604f1c23 > --- /dev/null > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > @@ -0,0 +1,13 @@ > +/dts-v1/; > +/plugin/; > + > +/ { > + fragment@0 { > + target-path = "/soc@0"; > + __overlay__ { > + usb@78d9000 { > + dr_mode = "host"; > + }; > + }; > + }; > +}; > -- > 2.40.1 Can we use normal dtso syntax here instead of defining fragments manually?
Hi, On Mon, Sep 04, 2023 at 07:59:26PM +0300, Dmitry Baryshkov wrote: > On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: > > > > Force db410c to host mode to fix network issue which results in failure > > to mount root fs via NFS. > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > > > Use fdtoverlay command to merge base device tree with an overlay > > which contains the fix for USB controllers to work in host mode. > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > > --- > > > > v2: > > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > > > --- > > drivers/gpu/drm/ci/build.sh | 5 +++++ > > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > > 2 files changed, 18 insertions(+) > > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > > index 7b014287a041..92ffd98cd09e 100644 > > --- a/drivers/gpu/drm/ci/build.sh > > +++ b/drivers/gpu/drm/ci/build.sh > > @@ -92,6 +92,11 @@ done > > > > if [[ -n ${DEVICE_TREES} ]]; then > > make dtbs > > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > > + fi > > cp ${DEVICE_TREES} /lava-files/. > > fi > > > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > new file mode 100644 > > index 000000000000..57b7604f1c23 > > --- /dev/null > > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > @@ -0,0 +1,13 @@ > > +/dts-v1/; > > +/plugin/; > > + > > +/ { > > + fragment@0 { > > + target-path = "/soc@0"; > > + __overlay__ { > > + usb@78d9000 { > > + dr_mode = "host"; > > + }; > > + }; > > + }; > > +}; > > -- > > 2.40.1 > > Can we use normal dtso syntax here instead of defining fragments manually? What Dmitry is hinting about is to use the "Sugar Syntax". There a good documentation here: https://source.android.com/docs/core/architecture/dto/syntax Maxime
Hi Dmitry, Maxime, On 05/09/23 14:13, Maxime Ripard wrote: > Hi, > > On Mon, Sep 04, 2023 at 07:59:26PM +0300, Dmitry Baryshkov wrote: >> On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: >>> >>> Force db410c to host mode to fix network issue which results in failure >>> to mount root fs via NFS. >>> See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 >>> >>> Use fdtoverlay command to merge base device tree with an overlay >>> which contains the fix for USB controllers to work in host mode. >>> >>> Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> >>> --- >>> >>> v2: >>> - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources >>> >>> --- >>> drivers/gpu/drm/ci/build.sh | 5 +++++ >>> .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ >>> 2 files changed, 18 insertions(+) >>> create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>> >>> diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh >>> index 7b014287a041..92ffd98cd09e 100644 >>> --- a/drivers/gpu/drm/ci/build.sh >>> +++ b/drivers/gpu/drm/ci/build.sh >>> @@ -92,6 +92,11 @@ done >>> >>> if [[ -n ${DEVICE_TREES} ]]; then >>> make dtbs >>> + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then >>> + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>> + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo >>> + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb >>> + fi >>> cp ${DEVICE_TREES} /lava-files/. >>> fi >>> >>> diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>> new file mode 100644 >>> index 000000000000..57b7604f1c23 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>> @@ -0,0 +1,13 @@ >>> +/dts-v1/; >>> +/plugin/; >>> + >>> +/ { >>> + fragment@0 { >>> + target-path = "/soc@0"; >>> + __overlay__ { >>> + usb@78d9000 { >>> + dr_mode = "host"; >>> + }; >>> + }; >>> + }; >>> +}; >>> -- >>> 2.40.1 >> >> Can we use normal dtso syntax here instead of defining fragments manually? > > What Dmitry is hinting about is to use the "Sugar Syntax". There a good documentation here: > https://source.android.com/docs/core/architecture/dto/syntax With the below DTO syntax, /dts-v1/; /plugin/; &usb { usb@78d9000 { dr_mode = "host"; }; }; Decoded dtbo file is, /dts-v1/; / { fragment@0 { target = <0xffffffff>; __overlay__ { usb@78d9000 { dr_mode = "host"; }; }; }; __fixups__ { usb = "/fragment@0:target:0"; }; }; With the previous fix using fragment we get, / { fragment@0 { target-path = "/soc@0"; __overlay__ { usb@78d9000 { dr_mode = "host"; }; }; }; }; Decoded apq8016-sbc.dtb file with the fix (setting dr_mode to host) is, /dts-v1/; / { soc@0 { usb@78d9000 { dr_mode = "host"; }; }; }; How can set the target to "soc@0" using the DTO syntax? Otherwise fdtoverlay fails to apply the dtbo file with the base dtb. Regards, Vignesh
On Tue, Sep 05, 2023 at 03:39:33PM +0530, Vignesh Raman wrote: > Hi Dmitry, Maxime, > > On 05/09/23 14:13, Maxime Ripard wrote: > > Hi, > > > > On Mon, Sep 04, 2023 at 07:59:26PM +0300, Dmitry Baryshkov wrote: > > > On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: > > > > > > > > Force db410c to host mode to fix network issue which results in failure > > > > to mount root fs via NFS. > > > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > > > > > > > Use fdtoverlay command to merge base device tree with an overlay > > > > which contains the fix for USB controllers to work in host mode. > > > > > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > > > > --- > > > > > > > > v2: > > > > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > > > > > > > --- > > > > drivers/gpu/drm/ci/build.sh | 5 +++++ > > > > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > > > > 2 files changed, 18 insertions(+) > > > > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > > > > index 7b014287a041..92ffd98cd09e 100644 > > > > --- a/drivers/gpu/drm/ci/build.sh > > > > +++ b/drivers/gpu/drm/ci/build.sh > > > > @@ -92,6 +92,11 @@ done > > > > > > > > if [[ -n ${DEVICE_TREES} ]]; then > > > > make dtbs > > > > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > > > > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > > > > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > > > > + fi > > > > cp ${DEVICE_TREES} /lava-files/. > > > > fi > > > > > > > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > new file mode 100644 > > > > index 000000000000..57b7604f1c23 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > @@ -0,0 +1,13 @@ > > > > +/dts-v1/; > > > > +/plugin/; > > > > + > > > > +/ { > > > > + fragment@0 { > > > > + target-path = "/soc@0"; > > > > + __overlay__ { > > > > + usb@78d9000 { > > > > + dr_mode = "host"; > > > > + }; > > > > + }; > > > > + }; > > > > +}; > > > > -- > > > > 2.40.1 > > > > > > Can we use normal dtso syntax here instead of defining fragments manually? > > > > What Dmitry is hinting about is to use the "Sugar Syntax". There a good documentation here: > > https://source.android.com/docs/core/architecture/dto/syntax > > > With the below DTO syntax, > /dts-v1/; > /plugin/; > > &usb { > usb@78d9000 { > dr_mode = "host"; > }; > }; > > Decoded dtbo file is, > /dts-v1/; > > / { > > fragment@0 { > target = <0xffffffff>; > > __overlay__ { > > usb@78d9000 { > dr_mode = "host"; > }; > }; > }; > > __fixups__ { > usb = "/fragment@0:target:0"; > }; > }; > > With the previous fix using fragment we get, > / { > > fragment@0 { > target-path = "/soc@0"; > > __overlay__ { > > usb@78d9000 { > dr_mode = "host"; > }; > }; > }; > }; > > Decoded apq8016-sbc.dtb file with the fix (setting dr_mode to host) is, > /dts-v1/; > / { > soc@0 { > usb@78d9000 { > dr_mode = "host"; > }; > }; > }; > > How can set the target to "soc@0" using the DTO syntax? To strictly answer your question, that would be something like &{/soc@0} { usb@78d9000 { dr_mode = "host"; }; }; You can simplify this further however by doing: &{/soc@0/usb@78d9000} { dr_mode = "host"; }; Also, that node actually has a label ("usb"), defined here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 So you can end up with &usb { dr_mode = "host"; }; All of them should be equivalent to the one you had in your patch. Maxime
On Tue, 5 Sept 2023 at 14:00, Maxime Ripard <mripard@kernel.org> wrote: > > On Tue, Sep 05, 2023 at 03:39:33PM +0530, Vignesh Raman wrote: > > Hi Dmitry, Maxime, > > > > On 05/09/23 14:13, Maxime Ripard wrote: > > > Hi, > > > > > > On Mon, Sep 04, 2023 at 07:59:26PM +0300, Dmitry Baryshkov wrote: > > > > On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: > > > > > > > > > > Force db410c to host mode to fix network issue which results in failure > > > > > to mount root fs via NFS. > > > > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > > > > > > > > > Use fdtoverlay command to merge base device tree with an overlay > > > > > which contains the fix for USB controllers to work in host mode. > > > > > > > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > > > > > --- > > > > > > > > > > v2: > > > > > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > > > > > > > > > --- > > > > > drivers/gpu/drm/ci/build.sh | 5 +++++ > > > > > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > > > > > 2 files changed, 18 insertions(+) > > > > > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > > > > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > > > > > index 7b014287a041..92ffd98cd09e 100644 > > > > > --- a/drivers/gpu/drm/ci/build.sh > > > > > +++ b/drivers/gpu/drm/ci/build.sh > > > > > @@ -92,6 +92,11 @@ done > > > > > > > > > > if [[ -n ${DEVICE_TREES} ]]; then > > > > > make dtbs > > > > > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > > > > > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > > > > > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > > > > > + fi > > > > > cp ${DEVICE_TREES} /lava-files/. > > > > > fi > > > > > > > > > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > > new file mode 100644 > > > > > index 000000000000..57b7604f1c23 > > > > > --- /dev/null > > > > > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > > @@ -0,0 +1,13 @@ > > > > > +/dts-v1/; > > > > > +/plugin/; > > > > > + > > > > > +/ { > > > > > + fragment@0 { > > > > > + target-path = "/soc@0"; > > > > > + __overlay__ { > > > > > + usb@78d9000 { > > > > > + dr_mode = "host"; > > > > > + }; > > > > > + }; > > > > > + }; > > > > > +}; > > > > > -- > > > > > 2.40.1 > > > > > > > > Can we use normal dtso syntax here instead of defining fragments manually? > > > > > > What Dmitry is hinting about is to use the "Sugar Syntax". There a good documentation here: > > > https://source.android.com/docs/core/architecture/dto/syntax > > > > > > With the below DTO syntax, > > /dts-v1/; > > /plugin/; > > > > &usb { > > usb@78d9000 { > > dr_mode = "host"; > > }; > > }; > > > > Decoded dtbo file is, > > /dts-v1/; > > > > / { > > > > fragment@0 { > > target = <0xffffffff>; > > > > __overlay__ { > > > > usb@78d9000 { > > dr_mode = "host"; > > }; > > }; > > }; > > > > __fixups__ { > > usb = "/fragment@0:target:0"; > > }; > > }; > > > > With the previous fix using fragment we get, > > / { > > > > fragment@0 { > > target-path = "/soc@0"; > > > > __overlay__ { > > > > usb@78d9000 { > > dr_mode = "host"; > > }; > > }; > > }; > > }; > > > > Decoded apq8016-sbc.dtb file with the fix (setting dr_mode to host) is, > > /dts-v1/; > > / { > > soc@0 { > > usb@78d9000 { > > dr_mode = "host"; > > }; > > }; > > }; > > > > How can set the target to "soc@0" using the DTO syntax? > > To strictly answer your question, that would be something like > > &{/soc@0} { > usb@78d9000 { > dr_mode = "host"; > }; > }; > > You can simplify this further however by doing: > > > &{/soc@0/usb@78d9000} { > dr_mode = "host"; > }; > > Also, that node actually has a label ("usb"), defined here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 > > So you can end up with > > &usb { > dr_mode = "host"; > }; ... which is the simplest and thus more robust one. > All of them should be equivalent to the one you had in your patch. > > Maxime
Hi, On 05/09/23 16:40, Dmitry Baryshkov wrote: > On Tue, 5 Sept 2023 at 14:00, Maxime Ripard <mripard@kernel.org> wrote: >> >> On Tue, Sep 05, 2023 at 03:39:33PM +0530, Vignesh Raman wrote: >>> Hi Dmitry, Maxime, >>> >>> On 05/09/23 14:13, Maxime Ripard wrote: >>>> Hi, >>>> >>>> On Mon, Sep 04, 2023 at 07:59:26PM +0300, Dmitry Baryshkov wrote: >>>>> On Mon, 4 Sept 2023 at 19:16, Vignesh Raman <vignesh.raman@collabora.com> wrote: >>>>>> >>>>>> Force db410c to host mode to fix network issue which results in failure >>>>>> to mount root fs via NFS. >>>>>> See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 >>>>>> >>>>>> Use fdtoverlay command to merge base device tree with an overlay >>>>>> which contains the fix for USB controllers to work in host mode. >>>>>> >>>>>> Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> >>>>>> --- >>>>>> >>>>>> v2: >>>>>> - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources >>>>>> >>>>>> --- >>>>>> drivers/gpu/drm/ci/build.sh | 5 +++++ >>>>>> .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ >>>>>> 2 files changed, 18 insertions(+) >>>>>> create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>>>>> >>>>>> diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh >>>>>> index 7b014287a041..92ffd98cd09e 100644 >>>>>> --- a/drivers/gpu/drm/ci/build.sh >>>>>> +++ b/drivers/gpu/drm/ci/build.sh >>>>>> @@ -92,6 +92,11 @@ done >>>>>> >>>>>> if [[ -n ${DEVICE_TREES} ]]; then >>>>>> make dtbs >>>>>> + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then >>>>>> + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>>>>> + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo >>>>>> + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb >>>>>> + fi >>>>>> cp ${DEVICE_TREES} /lava-files/. >>>>>> fi >>>>>> >>>>>> diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>>>>> new file mode 100644 >>>>>> index 000000000000..57b7604f1c23 >>>>>> --- /dev/null >>>>>> +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts >>>>>> @@ -0,0 +1,13 @@ >>>>>> +/dts-v1/; >>>>>> +/plugin/; >>>>>> + >>>>>> +/ { >>>>>> + fragment@0 { >>>>>> + target-path = "/soc@0"; >>>>>> + __overlay__ { >>>>>> + usb@78d9000 { >>>>>> + dr_mode = "host"; >>>>>> + }; >>>>>> + }; >>>>>> + }; >>>>>> +}; >>>>>> -- >>>>>> 2.40.1 >>>>> >>>>> Can we use normal dtso syntax here instead of defining fragments manually? >>>> >>>> What Dmitry is hinting about is to use the "Sugar Syntax". There a good documentation here: >>>> https://source.android.com/docs/core/architecture/dto/syntax >>> >>> >>> With the below DTO syntax, >>> /dts-v1/; >>> /plugin/; >>> >>> &usb { >>> usb@78d9000 { >>> dr_mode = "host"; >>> }; >>> }; >>> >>> Decoded dtbo file is, >>> /dts-v1/; >>> >>> / { >>> >>> fragment@0 { >>> target = <0xffffffff>; >>> >>> __overlay__ { >>> >>> usb@78d9000 { >>> dr_mode = "host"; >>> }; >>> }; >>> }; >>> >>> __fixups__ { >>> usb = "/fragment@0:target:0"; >>> }; >>> }; >>> >>> With the previous fix using fragment we get, >>> / { >>> >>> fragment@0 { >>> target-path = "/soc@0"; >>> >>> __overlay__ { >>> >>> usb@78d9000 { >>> dr_mode = "host"; >>> }; >>> }; >>> }; >>> }; >>> >>> Decoded apq8016-sbc.dtb file with the fix (setting dr_mode to host) is, >>> /dts-v1/; >>> / { >>> soc@0 { >>> usb@78d9000 { >>> dr_mode = "host"; >>> }; >>> }; >>> }; >>> >>> How can set the target to "soc@0" using the DTO syntax? >> >> To strictly answer your question, that would be something like >> >> &{/soc@0} { >> usb@78d9000 { >> dr_mode = "host"; >> }; >> }; >> >> You can simplify this further however by doing: >> >> >> &{/soc@0/usb@78d9000} { >> dr_mode = "host"; >> }; The above works. Thanks. >> >> Also, that node actually has a label ("usb"), defined here: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 >> >> So you can end up with >> >> &usb { >> dr_mode = "host"; >> }; > > ... which is the simplest and thus more robust one. > Should it be, &{/soc@0/usb} { dr_mode = "host"; }; I will send a v3 version for this. Thank you. Regards, Vignesh
On Tue, Sep 05, 2023 at 05:11:43PM +0530, Vignesh Raman wrote: > > > Also, that node actually has a label ("usb"), defined here: > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 > > > > > > So you can end up with > > > > > > &usb { > > > dr_mode = "host"; > > > }; > > > > ... which is the simplest and thus more robust one. > > > > Should it be, > &{/soc@0/usb} { > dr_mode = "host"; > }; No. The &{/...} syntax refers to a path. &... refers to a label. They are not equivalent. Maxime
Hi Maxime, On 05/09/23 17:27, Maxime Ripard wrote: > On Tue, Sep 05, 2023 at 05:11:43PM +0530, Vignesh Raman wrote: >>>> Also, that node actually has a label ("usb"), defined here: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 >>>> >>>> So you can end up with >>>> >>>> &usb { >>>> dr_mode = "host"; >>>> }; >>> >>> ... which is the simplest and thus more robust one. >>> >> >> Should it be, >> &{/soc@0/usb} { >> dr_mode = "host"; >> }; > > No. The &{/...} syntax refers to a path. &... refers to a label. They > are not equivalent. Sorry I was not clear before. With, &usb { dr_mode = "host"; }; The target is <0xffffffff> and fdtoverlay fails to apply the dtbo. With, &{/soc@0/usb} { dr_mode = "host"; }; The target-path is "/soc@0/usb" (usb: usb@78d9000) / { fragment@0 { target-path = "/soc@0/usb"; __overlay__ { dr_mode = "host"; }; }; }; So will use &{/...} syntax in this case. Regards, Vignesh
On Tue, Sep 05, 2023 at 07:06:43PM +0530, Vignesh Raman wrote: > Hi Maxime, > > On 05/09/23 17:27, Maxime Ripard wrote: > > On Tue, Sep 05, 2023 at 05:11:43PM +0530, Vignesh Raman wrote: > > > > > Also, that node actually has a label ("usb"), defined here: > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/msm8916.dtsi#n2322 > > > > > > > > > > So you can end up with > > > > > > > > > > &usb { > > > > > dr_mode = "host"; > > > > > }; > > > > > > > > ... which is the simplest and thus more robust one. > > > > > > > > > > Should it be, > > > &{/soc@0/usb} { > > > dr_mode = "host"; > > > }; > > > > No. The &{/...} syntax refers to a path. &... refers to a label. They > > are not equivalent. > > Sorry I was not clear before. > > With, > &usb { > dr_mode = "host"; > }; > > The target is <0xffffffff> and fdtoverlay fails to apply the dtbo. You do have /plugin/ and have compiled the base device tree with overlay support, right? > With, > &{/soc@0/usb} { > dr_mode = "host"; > }; > > The target-path is "/soc@0/usb" (usb: usb@78d9000) Right, and that's not the path you want to modify. The path you want to modify is /soc@0/usb@78d9000. usb is the label, it's absolute, and you can't mix and match a path ("/soc@0/") and a label ("usb") Maxime
Hi Maxime, On 05/09/23 19:10, Maxime Ripard wrote: >> With, >> &usb { >> dr_mode = "host"; >> }; >> >> The target is <0xffffffff> and fdtoverlay fails to apply the dtbo. > > You do have /plugin/ and have compiled the base device tree with overlay > support, right? After compiling base dtbs with overlay support (make DTC_FLAGS=-@ dtbs) it works. > >> With, >> &{/soc@0/usb} { >> dr_mode = "host"; >> }; >> >> The target-path is "/soc@0/usb" (usb: usb@78d9000) > > Right, and that's not the path you want to modify. The path you want to > modify is /soc@0/usb@78d9000. usb is the label, it's absolute, and you > can't mix and match a path ("/soc@0/") and a label ("usb") Thanks for the clarification. Regards, Vignesh
Hi! On 04/09/2023 13:15, Vignesh Raman wrote: > Force db410c to host mode to fix network issue which results in failure > to mount root fs via NFS. > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > Use fdtoverlay command to merge base device tree with an overlay > which contains the fix for USB controllers to work in host mode. > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > --- > > v2: > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > --- > drivers/gpu/drm/ci/build.sh | 5 +++++ > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > 2 files changed, 18 insertions(+) > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > index 7b014287a041..92ffd98cd09e 100644 > --- a/drivers/gpu/drm/ci/build.sh > +++ b/drivers/gpu/drm/ci/build.sh > @@ -92,6 +92,11 @@ done > > if [[ -n ${DEVICE_TREES} ]]; then > make dtbs > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > + fi > cp ${DEVICE_TREES} /lava-files/. > fi > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > new file mode 100644 > index 000000000000..57b7604f1c23 > --- /dev/null > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > @@ -0,0 +1,13 @@ > +/dts-v1/; > +/plugin/; > + > +/ { > + fragment@0 { > + target-path = "/soc@0"; > + __overlay__ { > + usb@78d9000 { > + dr_mode = "host"; > + }; > + }; > + }; > +}; Another thing that I was discussing with David and Vignesh, since we will need this overlay spinets not only for drm-ci but also for mesa ci (and every body who uses the farms), would it be interesting to move it to some place more official? like dts folders? Or would that be against Linux policy? Regards, Helen
On Wed, Sep 06, 2023 at 09:55:40AM -0300, Helen Koike wrote: > Hi! > > On 04/09/2023 13:15, Vignesh Raman wrote: > > Force db410c to host mode to fix network issue which results in failure > > to mount root fs via NFS. > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 > > > > Use fdtoverlay command to merge base device tree with an overlay > > which contains the fix for USB controllers to work in host mode. > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> > > --- > > > > v2: > > - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources > > --- > > drivers/gpu/drm/ci/build.sh | 5 +++++ > > .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ > > 2 files changed, 18 insertions(+) > > create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh > > index 7b014287a041..92ffd98cd09e 100644 > > --- a/drivers/gpu/drm/ci/build.sh > > +++ b/drivers/gpu/drm/ci/build.sh > > @@ -92,6 +92,11 @@ done > > if [[ -n ${DEVICE_TREES} ]]; then > > make dtbs > > + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then > > + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo > > + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb > > + fi > > cp ${DEVICE_TREES} /lava-files/. > > fi > > diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > new file mode 100644 > > index 000000000000..57b7604f1c23 > > --- /dev/null > > +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts > > @@ -0,0 +1,13 @@ > > +/dts-v1/; > > +/plugin/; > > + > > +/ { > > + fragment@0 { > > + target-path = "/soc@0"; > > + __overlay__ { > > + usb@78d9000 { > > + dr_mode = "host"; > > + }; > > + }; > > + }; > > +}; > > > Another thing that I was discussing with David and Vignesh, since we will > need this overlay spinets not only for drm-ci but also for mesa ci (and > every body who uses the farms), would it be interesting to move it to some > place more official? like dts folders? Or would that be against Linux > policy? AFAIK, the policy was changed recently to allow overlays to be merged, see $(find arch/ -name *.dtso). So generally speaking, it should be ok to send it. Maxime
diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh index 7b014287a041..92ffd98cd09e 100644 --- a/drivers/gpu/drm/ci/build.sh +++ b/drivers/gpu/drm/ci/build.sh @@ -92,6 +92,11 @@ done if [[ -n ${DEVICE_TREES} ]]; then make dtbs + if [[ -e arch/arm64/boot/dts/qcom/apq8016-sbc.dtb ]]; then + dtc -@ -I dts -O dtb -o drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts + fdtoverlay -i arch/arm64/boot/dts/qcom/apq8016-sbc.dtb -o arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dtbo + mv arch/arm64/boot/dts/qcom/apq8016-sbc-overlay.dtb arch/arm64/boot/dts/qcom/apq8016-sbc.dtb + fi cp ${DEVICE_TREES} /lava-files/. fi diff --git a/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts new file mode 100644 index 000000000000..57b7604f1c23 --- /dev/null +++ b/drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts @@ -0,0 +1,13 @@ +/dts-v1/; +/plugin/; + +/ { + fragment@0 { + target-path = "/soc@0"; + __overlay__ { + usb@78d9000 { + dr_mode = "host"; + }; + }; + }; +};
Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS. See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8 Use fdtoverlay command to merge base device tree with an overlay which contains the fix for USB controllers to work in host mode. Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com> --- v2: - Use fdtoverlay command to merge overlay dtbo with the base dtb instead of modifying the kernel sources --- drivers/gpu/drm/ci/build.sh | 5 +++++ .../gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts | 13 +++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 drivers/gpu/drm/ci/dt-overlays/apq8016-sbc-overlay.dts