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

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

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?

-- 
With best wishes
Dmitry

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

-- 
With best wishes
Dmitry

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