| Message ID | 20241204-topic-misc-dt-fixes-v1-2-6d320b6454e6@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: misc DT bindings check fixes | expand |
On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: > Bindings documents data-lanes as a single entry with a separate > clock-lanes property, but DT uses 2 entries in data-lanes. > > This would suggest clock-lanes is missing, fix the DT using the > bindings example. > > This fixes: > sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long > from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso > index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso > +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso > @@ -115,7 +115,8 @@ camera@60 { > > port { > ov7251_ep: endpoint { > - data-lanes = <0 1>; > + clock-lanes = <1>; > + data-lanes = <0>; Is it really this way or the other way around, clock = <0>, data = <1>? > /* remote-endpoint = <&csiphy3_ep>; */ > }; > }; > > -- > 2.34.1 >
On 04/12/2024 12:05, Dmitry Baryshkov wrote: > On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >> Bindings documents data-lanes as a single entry with a separate >> clock-lanes property, but DT uses 2 entries in data-lanes. >> >> This would suggest clock-lanes is missing, fix the DT using the >> bindings example. >> >> This fixes: >> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >> @@ -115,7 +115,8 @@ camera@60 { >> >> port { >> ov7251_ep: endpoint { >> - data-lanes = <0 1>; >> + clock-lanes = <1>; >> + data-lanes = <0>; > > Is it really this way or the other way around, clock = <0>, data = <1>? No idea actually, on the schematics the lanes from the DB845 are : CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC and CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC So I assume the data-lane is 0, for clock-lane I just used the example, but I found nothing in the code using those assignments Neil > >> /* remote-endpoint = <&csiphy3_ep>; */ >> }; >> }; >> >> -- >> 2.34.1 >> >
On 12/4/24 14:16, Neil Armstrong wrote: > On 04/12/2024 12:05, Dmitry Baryshkov wrote: >> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >>> Bindings documents data-lanes as a single entry with a separate >>> clock-lanes property, but DT uses 2 entries in data-lanes. >>> >>> This would suggest clock-lanes is missing, fix the DT using the >>> bindings example. >>> >>> This fixes: >>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >>> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>> @@ -115,7 +115,8 @@ camera@60 { >>> >>> port { >>> ov7251_ep: endpoint { >>> - data-lanes = <0 1>; >>> + clock-lanes = <1>; >>> + data-lanes = <0>; >> >> Is it really this way or the other way around, clock = <0>, data = <1>? > > No idea actually, on the schematics the lanes from the DB845 are : > > CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC > and > CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC > > So I assume the data-lane is 0, for clock-lane I just used > the example, but I found nothing in the code using those assignments > It's a sensor property, and OV7251 sensor has the non-selectable clock lane. If it's technically acceptable, I would rather suggest to deprecate and remove "clock-lanes" property and hard-code "data-lanes" value to <1>. By the way, this particular device tree bindings miss bus-type property, it could be either MIPI or LVDS serial output. -- Best wishes, Vladimir
On 04/12/2024 14:10, Vladimir Zapolskiy wrote: > On 12/4/24 14:16, Neil Armstrong wrote: >> On 04/12/2024 12:05, Dmitry Baryshkov wrote: >>> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >>>> Bindings documents data-lanes as a single entry with a separate >>>> clock-lanes property, but DT uses 2 entries in data-lanes. >>>> >>>> This would suggest clock-lanes is missing, fix the DT using the >>>> bindings example. >>>> >>>> This fixes: >>>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >>>> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >>>> >>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>> --- >>>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>> @@ -115,7 +115,8 @@ camera@60 { >>>> port { >>>> ov7251_ep: endpoint { >>>> - data-lanes = <0 1>; >>>> + clock-lanes = <1>; >>>> + data-lanes = <0>; >>> >>> Is it really this way or the other way around, clock = <0>, data = <1>? >> >> No idea actually, on the schematics the lanes from the DB845 are : >> >> CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC >> and >> CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC >> >> So I assume the data-lane is 0, for clock-lane I just used >> the example, but I found nothing in the code using those assignments >> > > It's a sensor property, and OV7251 sensor has the non-selectable clock lane. > > If it's technically acceptable, I would rather suggest to deprecate and > remove "clock-lanes" property and hard-code "data-lanes" value to <1>. Ok indeed while looking at the OV7251 sensor datasheet, there's a single fixed data lane and a single fixed clock lane, so on the sensor side we can't select anything but how would we define lane0 is used on the SoC side ? > > By the way, this particular device tree bindings miss bus-type property, > it could be either MIPI or LVDS serial output. > > -- > Best wishes, > Vladimir
On 12/4/24 15:16, neil.armstrong@linaro.org wrote: > On 04/12/2024 14:10, Vladimir Zapolskiy wrote: >> On 12/4/24 14:16, Neil Armstrong wrote: >>> On 04/12/2024 12:05, Dmitry Baryshkov wrote: >>>> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >>>>> Bindings documents data-lanes as a single entry with a separate >>>>> clock-lanes property, but DT uses 2 entries in data-lanes. >>>>> >>>>> This would suggest clock-lanes is missing, fix the DT using the >>>>> bindings example. >>>>> >>>>> This fixes: >>>>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >>>>> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >>>>> >>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>> --- >>>>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>> @@ -115,7 +115,8 @@ camera@60 { >>>>> port { >>>>> ov7251_ep: endpoint { >>>>> - data-lanes = <0 1>; >>>>> + clock-lanes = <1>; >>>>> + data-lanes = <0>; >>>> >>>> Is it really this way or the other way around, clock = <0>, data = <1>? >>> >>> No idea actually, on the schematics the lanes from the DB845 are : >>> >>> CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC >>> and >>> CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC >>> >>> So I assume the data-lane is 0, for clock-lane I just used >>> the example, but I found nothing in the code using those assignments >>> >> >> It's a sensor property, and OV7251 sensor has the non-selectable clock lane. >> >> If it's technically acceptable, I would rather suggest to deprecate and >> remove "clock-lanes" property and hard-code "data-lanes" value to <1>. > > > Ok indeed while looking at the OV7251 sensor datasheet, there's a single > fixed data lane and a single fixed clock lane, so on the sensor side we > can't select anything but how would we define lane0 is used on the SoC side ? It's done right in the common way, there are clock-lanes and data-lanes properties on the ISP endpoint side. >> >> By the way, this particular device tree bindings miss bus-type property, >> it could be either MIPI or LVDS serial output. >>
On 04/12/2024 14:30, Vladimir Zapolskiy wrote: > On 12/4/24 15:16, neil.armstrong@linaro.org wrote: >> On 04/12/2024 14:10, Vladimir Zapolskiy wrote: >>> On 12/4/24 14:16, Neil Armstrong wrote: >>>> On 04/12/2024 12:05, Dmitry Baryshkov wrote: >>>>> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >>>>>> Bindings documents data-lanes as a single entry with a separate >>>>>> clock-lanes property, but DT uses 2 entries in data-lanes. >>>>>> >>>>>> This would suggest clock-lanes is missing, fix the DT using the >>>>>> bindings example. >>>>>> >>>>>> This fixes: >>>>>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >>>>>> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >>>>>> >>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>>> --- >>>>>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>> @@ -115,7 +115,8 @@ camera@60 { >>>>>> port { >>>>>> ov7251_ep: endpoint { >>>>>> - data-lanes = <0 1>; >>>>>> + clock-lanes = <1>; >>>>>> + data-lanes = <0>; >>>>> >>>>> Is it really this way or the other way around, clock = <0>, data = <1>? >>>> >>>> No idea actually, on the schematics the lanes from the DB845 are : >>>> >>>> CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC >>>> and >>>> CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC >>>> >>>> So I assume the data-lane is 0, for clock-lane I just used >>>> the example, but I found nothing in the code using those assignments >>>> >>> >>> It's a sensor property, and OV7251 sensor has the non-selectable clock lane. >>> >>> If it's technically acceptable, I would rather suggest to deprecate and >>> remove "clock-lanes" property and hard-code "data-lanes" value to <1>. >> >> >> Ok indeed while looking at the OV7251 sensor datasheet, there's a single >> fixed data lane and a single fixed clock lane, so on the sensor side we >> can't select anything but how would we define lane0 is used on the SoC side ? > > It's done right in the common way, there are clock-lanes and data-lanes > properties on the ISP endpoint side. For the ov8856 yes, but in the current state the ov7251 endpoint is not connected to the csiphy3, so it's currently not usable and disabled, so I'd rather remove this node completely instead of fixing an untestable dtso. Neil > >>> >>> By the way, this particular device tree bindings miss bus-type property, >>> it could be either MIPI or LVDS serial output. >>> >
On 12/5/24 11:27, neil.armstrong@linaro.org wrote: > On 04/12/2024 14:30, Vladimir Zapolskiy wrote: >> On 12/4/24 15:16, neil.armstrong@linaro.org wrote: >>> On 04/12/2024 14:10, Vladimir Zapolskiy wrote: >>>> On 12/4/24 14:16, Neil Armstrong wrote: >>>>> On 04/12/2024 12:05, Dmitry Baryshkov wrote: >>>>>> On Wed, Dec 04, 2024 at 11:56:54AM +0100, Neil Armstrong wrote: >>>>>>> Bindings documents data-lanes as a single entry with a separate >>>>>>> clock-lanes property, but DT uses 2 entries in data-lanes. >>>>>>> >>>>>>> This would suggest clock-lanes is missing, fix the DT using the >>>>>>> bindings example. >>>>>>> >>>>>>> This fixes: >>>>>>> sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long >>>>>>> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# >>>>>>> >>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>>>> --- >>>>>>> arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>>> index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 >>>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso >>>>>>> @@ -115,7 +115,8 @@ camera@60 { >>>>>>> port { >>>>>>> ov7251_ep: endpoint { >>>>>>> - data-lanes = <0 1>; >>>>>>> + clock-lanes = <1>; >>>>>>> + data-lanes = <0>; >>>>>> >>>>>> Is it really this way or the other way around, clock = <0>, data = <1>? >>>>> >>>>> No idea actually, on the schematics the lanes from the DB845 are : >>>>> >>>>> CSI0_P/N -> OV7251_CSI3_LANE0_P/N -> MIPI_CSI3_LANE0_P -> SoC >>>>> and >>>>> CLKP/N -> OV7251_CSI3_CLK_P/N -> MIPI_CSI3_CLK_P/N -> SoC >>>>> >>>>> So I assume the data-lane is 0, for clock-lane I just used >>>>> the example, but I found nothing in the code using those assignments >>>>> >>>> >>>> It's a sensor property, and OV7251 sensor has the non-selectable clock lane. >>>> >>>> If it's technically acceptable, I would rather suggest to deprecate and >>>> remove "clock-lanes" property and hard-code "data-lanes" value to <1>. >>> >>> >>> Ok indeed while looking at the OV7251 sensor datasheet, there's a single >>> fixed data lane and a single fixed clock lane, so on the sensor side we >>> can't select anything but how would we define lane0 is used on the SoC side ? >> >> It's done right in the common way, there are clock-lanes and data-lanes >> properties on the ISP endpoint side. > > For the ov8856 yes, but in the current state the ov7251 endpoint is not Above I gave a generic description of describing the lanes. > connected to the csiphy3, so it's currently not usable and disabled, > so I'd rather remove this node completely instead of fixing an > untestable dtso. Sure, if the device is unused, it shall be removed. -- Best wishes, Vladimir
diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso index 0a87df806cafc8e726aacc07a772ca478d0ee3df..5a16f4c2b346b314af3d614266e1ca034057e643 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso @@ -115,7 +115,8 @@ camera@60 { port { ov7251_ep: endpoint { - data-lanes = <0 1>; + clock-lanes = <1>; + data-lanes = <0>; /* remote-endpoint = <&csiphy3_ep>; */ }; };
Bindings documents data-lanes as a single entry with a separate clock-lanes property, but DT uses 2 entries in data-lanes. This would suggest clock-lanes is missing, fix the DT using the bindings example. This fixes: sdm845-db845c-navigation-mezzanine.dtso: camera@60: port:endpoint:data-lanes: [0, 1] is too long from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov7251.yaml# Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- arch/arm64/boot/dts/qcom/sdm845-db845c-navigation-mezzanine.dtso | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)