| Message ID | 20230905233118.183140-4-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] dt-bindings: media: Add THine THP7312 ISP | expand |
Hi Krzysztof, On Wed, Sep 06, 2023 at 09:27:07AM +0200, Krzysztof Kozlowski wrote: > On 06/09/2023 01:31, Paul Elder wrote: > > Add overlays for the Pumpkin i350 to support THP7312 cameras. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > arch/arm64/boot/dts/mediatek/Makefile | 4 + > > .../mt8365-pumpkin-common-thp7312.dtsi | 23 ++++++ > > .../mt8365-pumpkin-csi0-thp7312-imx258.dtso | 73 +++++++++++++++++++ > > .../mt8365-pumpkin-csi1-thp7312-imx258.dtso | 73 +++++++++++++++++++ > > 4 files changed, 173 insertions(+) > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso > > > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile > > index 20570bc40de8..ceaf24105001 100644 > > --- a/arch/arm64/boot/dts/mediatek/Makefile > > +++ b/arch/arm64/boot/dts/mediatek/Makefile > > @@ -56,4 +56,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-pumpkin.dtb > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb > > > > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi0-thp7312-imx258.dtbo > > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi1-thp7312-imx258.dtbo > > mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo > > + > > +dtb-$(CONFIG_ARCH_MEDIATEK) += mtk-mt8365-pumpkin.dtb > > diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi > > new file mode 100644 > > index 000000000000..478697552617 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi > > @@ -0,0 +1,23 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2023 Ideas on Board > > + * Author: Paul Elder <paul.elder@ideasonboard.com> > > + */ > > + > > +/dts-v1/; > > +/plugin/; > > + > > +&{/} { > > + vsys_v4p2: regulator@0 { > > Hm? Is this a bus? There are multiple instances of "numbered" regulators in upstream DT files, for instance arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi has a regulator@0. There are similar instances for clocks. I understand why it may not be a good idea, and how the root node is indeed not a bus. In some cases, those regulators and clocks are grouped in a regulators or clocks node that has a "simple-bus" compatible. I'm not sure if that's a good idea, but at least it should validate. What's the best practice for discrete board-level clocks and regulators in overlays ? How do we ensure that their node name will not conflict with the board to which the overlay is attached ? > > + compatible = "regulator-fixed"; > > + regulator-name = "vsys-v4p2"; > > + regulator-min-microvolt = <4200000>; > > + regulator-max-microvolt = <4200000>; > > + }; > > + > > + camera61_clk: cam_clk24m { > > And this is not on a bus? It's the same / node! > > Please work on mainline, which means take mainline code and change it to > your needs. Do not take downstream poor code and change it... > > No underscores in node names. Also generic node names, so at least with > generic prefix or suffix. > > > + compatible = "fixed-clock"; > > + clock-frequency = <24000000>; > > + #clock-cells = <0>; > > + }; > > +}; > > diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso > > new file mode 100644 > > index 000000000000..740d14a19d75 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso > > @@ -0,0 +1,73 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2023 Ideas on Board > > + * Author: Paul Elder <paul.elder@ideasonboard.com> > > + */ > > + > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/pinctrl/mt8365-pinfunc.h> > > +#include "mt8365-pumpkin-common-thp7312.dtsi" > > + > > +&i2c3 { > > + camera@61 { > > + compatible = "thine,thp7312"; > > + reg = <0x61>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&cam0_pins_default>; > > + reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>; > > + clocks = <&camera61_clk>; > > + > > + vddcore-supply = <&vsys_v4p2>; > > + vhtermrx-supply = <&vsys_v4p2>; > > + vddtx-supply = <&vsys_v4p2>; > > + vddhost-supply = <&vsys_v4p2>; > > + vddcmos-supply = <&vsys_v4p2>; > > + vddgpio_0-supply = <&vsys_v4p2>; > > + vddgpio_1-supply = <&vsys_v4p2>; > > + DOVDD-supply = <&vsys_v4p2>; > > + AVDD-supply = <&vsys_v4p2>; > > + DVDD-supply = <&vsys_v4p2>; > > + > > + orientation = <0>; > > + rotation = <0>; > > + > > + thine,rx,data-lanes = <4 1 3 2>; > > NAK for this property. Please explain why. You commented very briefly in the bindings review, and it wasn't clear to me if you were happy or not with the property, and if not, why. > > + > > + port { > > + isp1_out: endpoint { > > + remote-endpoint = <&seninf_in1>; > > + data-lanes = <4 2 1 3>; > > + }; > > + }; > > + }; > > +}; > > + > > +&pio { > > + cam0_pins_default: cam0_pins_default { > > No underscores in node names. > > > + pins_rst { > > Ditto
On Wed, Sep 06, 2023 at 11:21:31AM +0200, Krzysztof Kozlowski wrote: > On 06/09/2023 11:00, Laurent Pinchart wrote: > >>> has a regulator@0. There are similar instances for clocks. > >>> > >>> I understand why it may not be a good idea, and how the root node is > >>> indeed not a bus. In some cases, those regulators and clocks are grouped > >>> in a regulators or clocks node that has a "simple-bus" compatible. I'm > >>> not sure if that's a good idea, but at least it should validate. > >>> > >>> What's the best practice for discrete board-level clocks and regulators > >>> in overlays ? How do we ensure that their node name will not conflict > >>> with the board to which the overlay is attached ? > >> > >> Top-level nodes (so under /) do not have unit addresses. If they have - > >> it's an error, because it is not a bus. Also, unit address requires reg. > >> No reg? No unit address. DTC reports this as warnings as well. > > > > I agree with all that, but what's the recommended practice to add > > top-level clocks and regulators in overlays, in a way that avoids > > namespace clashes with the base board ? > > Whether you use regulator@0 or regulator-0, you have the same chances of > clash. No disagreement there. My question is whether there's a recommended practice to avoid clashes, or if it's an unsolved problem that gets ignored for now because there's only 36h in a day and there are more urgent things to do. > >>>>> + orientation = <0>; > >>>>> + rotation = <0>; > >>>>> + > >>>>> + thine,rx,data-lanes = <4 1 3 2>; > >>>> > >>>> NAK for this property. > >>> > >>> Please explain why. You commented very briefly in the bindings review, > >>> and it wasn't clear to me if you were happy or not with the property, > >>> and if not, why. > >> > >> Because it is duplicating endpoint. At least from the description. > > > > The THP7312 is an external ISP. At the hardware level, it has an input > > side, with a CSI-2 receiver and an I2C master controller, and an output > > side, with a CSI-2 transmitter and an I2C slave controller. A raw camera > > sensor is connected on the input side, transmitting image data to the > > THP7312, and being controlled over I2C by the firmware running on the > > THP7312. From a Linux point of view, only the output side of the THP7312 > > is visible, and the combination of the raw camera sensor and the THP7312 > > acts as a smart camera sensor, producing YUV images. > > None of this was explained in the device description or property field. I agree this can be improved. Paul, can you expand the description to make it clearer in the next version ? > I probably judged to fast but it just looked like duplicated property. > Then shouldn't it have two ports, even if camera side is not visible for > the Linux? I'm in two minds about this. On one hand, using ports means we can reuse standard properties, as well as helper code in the kernel, which is nice. On the other hand, it means we would also need to add a DT node to model the sensor, but the sensor isn't exposed to Linux, so we don't want that node to cause a device being instantiated. I think we'll need to add more properties related to the camera sensor in the future. Coupled with the fact that the THP7312 actually has two inputs to support two sensors at the same time (which neither the bindings nor the driver curently support, but that's fine, they can be added later), it would be nice to group all properties related to a particular THP7312 input in a node. I've given this a try for the AP1302 (another external ISP) a while ago. The bindings have been posted in https://lore.kernel.org/linux-media/20211006113254.3470-2-anil.mamidala@xilinx.com/. It still doesn't connect the sensors to the ISP in DT, but it nicely groups all sensor-related properties together. Is this something that you would be happier with ? > > As there are two CSI-2 buses, the data lanes configuration needs to be > > specified for both sides. On the output side, connected to the SoC and > > visible to Linux, the bindings use a port node with an endpoint and the > > standard data-lanes property. On the input side, which is invisible to > > Linux, the bindings use the vendor-specific thine,rx,data-lanes > > property. Its semantics is identical to the standard data-lanes > > property, but it's not located in an endpoint as there's no port for the > > input side. > > And how does the property support multiple sensors? What if they data > lanes are also different between each other? Ignoring for a moment that the THP7312 has two inputs, there would be no problem I think, as only one sensor is connected to the input. Different sensor models can be used on different boards, but only one at a time. To support the second input, we could add a thine,rx2,data-lanes property. It's not great, but not that bad either if it stopped there. However, if we later have to add additional sensor-related properties (such as regulators for instance), it could become ugly. Grouping sensor-related properties in child sensor nodes would be nicer I believe.
On Wed, Sep 06, 2023 at 12:01:43PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2023-09-06 10:35:31) > > On Wed, Sep 06, 2023 at 11:21:31AM +0200, Krzysztof Kozlowski wrote: > > > On 06/09/2023 11:00, Laurent Pinchart wrote: > > > >>> has a regulator@0. There are similar instances for clocks. > > > >>> > > > >>> I understand why it may not be a good idea, and how the root node is > > > >>> indeed not a bus. In some cases, those regulators and clocks are grouped > > > >>> in a regulators or clocks node that has a "simple-bus" compatible. I'm > > > >>> not sure if that's a good idea, but at least it should validate. > > > >>> > > > >>> What's the best practice for discrete board-level clocks and regulators > > > >>> in overlays ? How do we ensure that their node name will not conflict > > > >>> with the board to which the overlay is attached ? > > > >> > > > >> Top-level nodes (so under /) do not have unit addresses. If they have - > > > >> it's an error, because it is not a bus. Also, unit address requires reg. > > > >> No reg? No unit address. DTC reports this as warnings as well. > > > > > > > > I agree with all that, but what's the recommended practice to add > > > > top-level clocks and regulators in overlays, in a way that avoids > > > > namespace clashes with the base board ? > > > > > > Whether you use regulator@0 or regulator-0, you have the same chances of > > > clash. > > > > No disagreement there. My question is whether there's a recommended > > practice to avoid clashes, or if it's an unsolved problem that gets > > ignored for now because there's only 36h in a day and there are more > > urgent things to do. > > Should an overlay add these items to a simple-bus specific to that > overlay/device that is being supported? > > That would 'namespace' the added fixed-clocks/fixed-regulators etc... > > But maybe it's overengineering or mis-using the simple-bus. You would still need to name the node that groups the regulators and clocks in a way that wouldn't clash between multiple overlays and the base board. It would be nice to have nodes that are "private" to an overlay. > And the items are still not on a 'bus' with an address - they just exist > on a presumably externally provided board....
On Wed, Sep 06, 2023 at 02:14:29PM +0300, Laurent Pinchart wrote: > On Wed, Sep 06, 2023 at 12:01:43PM +0100, Kieran Bingham wrote: > > Quoting Laurent Pinchart (2023-09-06 10:35:31) > > > On Wed, Sep 06, 2023 at 11:21:31AM +0200, Krzysztof Kozlowski wrote: > > > > On 06/09/2023 11:00, Laurent Pinchart wrote: > > > > >>> has a regulator@0. There are similar instances for clocks. > > > > >>> > > > > >>> I understand why it may not be a good idea, and how the root node is > > > > >>> indeed not a bus. In some cases, those regulators and clocks are grouped > > > > >>> in a regulators or clocks node that has a "simple-bus" compatible. I'm > > > > >>> not sure if that's a good idea, but at least it should validate. > > > > >>> > > > > >>> What's the best practice for discrete board-level clocks and regulators > > > > >>> in overlays ? How do we ensure that their node name will not conflict > > > > >>> with the board to which the overlay is attached ? > > > > >> > > > > >> Top-level nodes (so under /) do not have unit addresses. If they have - > > > > >> it's an error, because it is not a bus. Also, unit address requires reg. > > > > >> No reg? No unit address. DTC reports this as warnings as well. > > > > > > > > > > I agree with all that, but what's the recommended practice to add > > > > > top-level clocks and regulators in overlays, in a way that avoids > > > > > namespace clashes with the base board ? > > > > > > > > Whether you use regulator@0 or regulator-0, you have the same chances of > > > > clash. > > > > > > No disagreement there. My question is whether there's a recommended > > > practice to avoid clashes, or if it's an unsolved problem that gets > > > ignored for now because there's only 36h in a day and there are more > > > urgent things to do. > > > > Should an overlay add these items to a simple-bus specific to that > > overlay/device that is being supported? > > > > That would 'namespace' the added fixed-clocks/fixed-regulators etc... > > > > But maybe it's overengineering or mis-using the simple-bus. > > You would still need to name the node that groups the regulators and > clocks in a way that wouldn't clash between multiple overlays and the > base board. It would be nice to have nodes that are "private" to an > overlay. What's the best solution to this then :/ Paul > > > And the items are still not on a 'bus' with an address - they just exist > > on a presumably externally provided board....
On Thu, Sep 07, 2023 at 11:55:13PM +0900, Paul Elder wrote: > On Wed, Sep 06, 2023 at 02:14:29PM +0300, Laurent Pinchart wrote: > > On Wed, Sep 06, 2023 at 12:01:43PM +0100, Kieran Bingham wrote: > > > Quoting Laurent Pinchart (2023-09-06 10:35:31) > > > > On Wed, Sep 06, 2023 at 11:21:31AM +0200, Krzysztof Kozlowski wrote: > > > > > On 06/09/2023 11:00, Laurent Pinchart wrote: > > > > > >>> has a regulator@0. There are similar instances for clocks. > > > > > >>> > > > > > >>> I understand why it may not be a good idea, and how the root node is > > > > > >>> indeed not a bus. In some cases, those regulators and clocks are grouped > > > > > >>> in a regulators or clocks node that has a "simple-bus" compatible. I'm > > > > > >>> not sure if that's a good idea, but at least it should validate. > > > > > >>> > > > > > >>> What's the best practice for discrete board-level clocks and regulators > > > > > >>> in overlays ? How do we ensure that their node name will not conflict > > > > > >>> with the board to which the overlay is attached ? > > > > > >> > > > > > >> Top-level nodes (so under /) do not have unit addresses. If they have - > > > > > >> it's an error, because it is not a bus. Also, unit address requires reg. > > > > > >> No reg? No unit address. DTC reports this as warnings as well. > > > > > > > > > > > > I agree with all that, but what's the recommended practice to add > > > > > > top-level clocks and regulators in overlays, in a way that avoids > > > > > > namespace clashes with the base board ? > > > > > > > > > > Whether you use regulator@0 or regulator-0, you have the same chances of > > > > > clash. > > > > > > > > No disagreement there. My question is whether there's a recommended > > > > practice to avoid clashes, or if it's an unsolved problem that gets > > > > ignored for now because there's only 36h in a day and there are more > > > > urgent things to do. > > > > > > Should an overlay add these items to a simple-bus specific to that > > > overlay/device that is being supported? > > > > > > That would 'namespace' the added fixed-clocks/fixed-regulators etc... > > > > > > But maybe it's overengineering or mis-using the simple-bus. > > > > You would still need to name the node that groups the regulators and > > clocks in a way that wouldn't clash between multiple overlays and the > > base board. It would be nice to have nodes that are "private" to an > > overlay. > > What's the best solution to this then :/ It seems we don't have a good solution. For now, I'd recommend just picking a name for the regulator that has a high chance to be unique, like reg-thp7312-1v2 for instance. > > > And the items are still not on a 'bus' with an address - they just exist > > > on a presumably externally provided board....
On Wed, Sep 06, 2023 at 08:31:18AM +0900, Paul Elder wrote: > Add overlays for the Pumpkin i350 to support THP7312 cameras. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > arch/arm64/boot/dts/mediatek/Makefile | 4 + > .../mt8365-pumpkin-common-thp7312.dtsi | 23 ++++++ > .../mt8365-pumpkin-csi0-thp7312-imx258.dtso | 73 +++++++++++++++++++ > .../mt8365-pumpkin-csi1-thp7312-imx258.dtso | 73 +++++++++++++++++++ > 4 files changed, 173 insertions(+) > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile > index 20570bc40de8..ceaf24105001 100644 > --- a/arch/arm64/boot/dts/mediatek/Makefile > +++ b/arch/arm64/boot/dts/mediatek/Makefile > @@ -56,4 +56,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-pumpkin.dtb > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb > > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi0-thp7312-imx258.dtbo > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi1-thp7312-imx258.dtbo > mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo This has no effect. You are assigning the same variable 3 times. It needs to be every overlay applied to its base is a *-dtbs variable and then those are all added to 'dtb-y'. IOW, we don't allow overlays which can't be applied at build time. Assuming the overlays aren't mutually exclusive, you could do: mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo mtk-mt8365-pumpkin-dtbs += mt8365-pumpkin-csi0-thp7312-imx258.dtbo mtk-mt8365-pumpkin-dtbs += mt8365-pumpkin-csi1-thp7312-imx258.dtbo This works the same way as '-objs' variables to group .o files into a module. > + > +dtb-$(CONFIG_ARCH_MEDIATEK) += mtk-mt8365-pumpkin.dtb Looks like mtk-mt8365-pumpkin.dtb failed to get built before. In any case, that's a terrible name. What's the difference between mt8365-pumpkin.dtb and mtk-mt8365-pumpkin.dtb? There's no clue. Rob
On Fri, Sep 08, 2023 at 03:52:22PM -0500, Rob Herring wrote: > On Wed, Sep 06, 2023 at 08:31:18AM +0900, Paul Elder wrote: > > Add overlays for the Pumpkin i350 to support THP7312 cameras. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > arch/arm64/boot/dts/mediatek/Makefile | 4 + > > .../mt8365-pumpkin-common-thp7312.dtsi | 23 ++++++ > > .../mt8365-pumpkin-csi0-thp7312-imx258.dtso | 73 +++++++++++++++++++ > > .../mt8365-pumpkin-csi1-thp7312-imx258.dtso | 73 +++++++++++++++++++ > > 4 files changed, 173 insertions(+) > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso > > create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso > > > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile > > index 20570bc40de8..ceaf24105001 100644 > > --- a/arch/arm64/boot/dts/mediatek/Makefile > > +++ b/arch/arm64/boot/dts/mediatek/Makefile > > @@ -56,4 +56,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-pumpkin.dtb > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb > > > > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi0-thp7312-imx258.dtbo > > +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi1-thp7312-imx258.dtbo > > mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo > > This has no effect. You are assigning the same variable 3 times. It > needs to be every overlay applied to its base is a *-dtbs variable and > then those are all added to 'dtb-y'. IOW, we don't allow overlays which > can't be applied at build time. > > Assuming the overlays aren't mutually exclusive, you could do: > > mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo > mtk-mt8365-pumpkin-dtbs += mt8365-pumpkin-csi0-thp7312-imx258.dtbo > mtk-mt8365-pumpkin-dtbs += mt8365-pumpkin-csi1-thp7312-imx258.dtbo > > This works the same way as '-objs' variables to group .o files into a > module. > > > + > > +dtb-$(CONFIG_ARCH_MEDIATEK) += mtk-mt8365-pumpkin.dtb > > Looks like mtk-mt8365-pumpkin.dtb failed to get built before. In any > case, that's a terrible name. What's the difference between > mt8365-pumpkin.dtb and mtk-mt8365-pumpkin.dtb? There's no clue. That's a bad name indeed. The cover letter indicates that this patch depends on currently out-of-tree DT changes, including a work-in-progress commit that adds mt8365-pumpkin-ethernet-usb.dtbo. The commits order makes this patch look weird. Paul, in your v2, please indicate in the commit message of this patch that it isn't intended to be merged yet. A "[DNI]" prefix in the subject line will help.
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile index 20570bc40de8..ceaf24105001 100644 --- a/arch/arm64/boot/dts/mediatek/Makefile +++ b/arch/arm64/boot/dts/mediatek/Makefile @@ -56,4 +56,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-pumpkin.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi0-thp7312-imx258.dtbo +mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-csi1-thp7312-imx258.dtbo mtk-mt8365-pumpkin-dtbs := mt8365-pumpkin.dtb mt8365-pumpkin-ethernet-usb.dtbo + +dtb-$(CONFIG_ARCH_MEDIATEK) += mtk-mt8365-pumpkin.dtb diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi new file mode 100644 index 000000000000..478697552617 --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2023 Ideas on Board + * Author: Paul Elder <paul.elder@ideasonboard.com> + */ + +/dts-v1/; +/plugin/; + +&{/} { + vsys_v4p2: regulator@0 { + compatible = "regulator-fixed"; + regulator-name = "vsys-v4p2"; + regulator-min-microvolt = <4200000>; + regulator-max-microvolt = <4200000>; + }; + + camera61_clk: cam_clk24m { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + #clock-cells = <0>; + }; +}; diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso new file mode 100644 index 000000000000..740d14a19d75 --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2023 Ideas on Board + * Author: Paul Elder <paul.elder@ideasonboard.com> + */ + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/pinctrl/mt8365-pinfunc.h> +#include "mt8365-pumpkin-common-thp7312.dtsi" + +&i2c3 { + camera@61 { + compatible = "thine,thp7312"; + reg = <0x61>; + pinctrl-names = "default"; + pinctrl-0 = <&cam0_pins_default>; + reset-gpios = <&pio 118 GPIO_ACTIVE_LOW>; + clocks = <&camera61_clk>; + + vddcore-supply = <&vsys_v4p2>; + vhtermrx-supply = <&vsys_v4p2>; + vddtx-supply = <&vsys_v4p2>; + vddhost-supply = <&vsys_v4p2>; + vddcmos-supply = <&vsys_v4p2>; + vddgpio_0-supply = <&vsys_v4p2>; + vddgpio_1-supply = <&vsys_v4p2>; + DOVDD-supply = <&vsys_v4p2>; + AVDD-supply = <&vsys_v4p2>; + DVDD-supply = <&vsys_v4p2>; + + orientation = <0>; + rotation = <0>; + + thine,rx,data-lanes = <4 1 3 2>; + + port { + isp1_out: endpoint { + remote-endpoint = <&seninf_in1>; + data-lanes = <4 2 1 3>; + }; + }; + }; +}; + +&pio { + cam0_pins_default: cam0_pins_default { + pins_rst { + pinmux = <MT8365_PIN_118_DMIC0_DAT0__FUNC_GPIO118>; + }; + }; +}; + +&seninf { + status = "okay"; + + ports { + port@0 { + seninf_in1: endpoint { + remote-endpoint = <&isp1_out>; + clock-lanes = <2>; + data-lanes = <1 3 0 4>; + }; + }; + }; +}; + +&camsv1 { + status = "okay"; +}; + +&mipi_csi0 { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso new file mode 100644 index 000000000000..2ebe4e9b56fa --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2023 Ideas on Board + * Author: Paul Elder <paul.elder@ideasonboard.com> + */ + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/pinctrl/mt8365-pinfunc.h> +#include "mt8365-pumpkin-common-thp7312.dtsi" + +&i2c2 { + camera@61 { + compatible = "thine,thp7312"; + reg = <0x61>; + pinctrl-names = "default"; + pinctrl-0 = <&cam1_pins_default>; + reset-gpios = <&pio 119 GPIO_ACTIVE_LOW>; + clocks = <&camera61_clk>; + + vddcore-supply = <&vsys_v4p2>; + vhtermrx-supply = <&vsys_v4p2>; + vddtx-supply = <&vsys_v4p2>; + vddhost-supply = <&vsys_v4p2>; + vddcmos-supply = <&vsys_v4p2>; + vddgpio_0-supply = <&vsys_v4p2>; + vddgpio_1-supply = <&vsys_v4p2>; + DOVDD-supply = <&vsys_v4p2>; + AVDD-supply = <&vsys_v4p2>; + DVDD-supply = <&vsys_v4p2>; + + orientation = <0>; + rotation = <0>; + + thine,rx,data-lanes = <4 1 3 2>; + + port { + isp2_out: endpoint { + remote-endpoint = <&seninf_in2>; + data-lanes = <4 2 1 3>; + }; + }; + }; +}; + +&pio { + cam1_pins_default: cam1_pins_default { + pins_rst { + pinmux = <MT8365_PIN_119_DMIC0_DAT1__FUNC_GPIO119>; + }; + }; +}; + +&seninf { + status = "okay"; + + ports { + port@1 { + seninf_in2: endpoint { + remote-endpoint = <&isp2_out>; + clock-lanes = <2>; + data-lanes = <1 3 0 4>; + }; + }; + }; +}; + +&camsv2 { + status = "okay"; +}; + +&mipi_csi1 { + status = "okay"; +};
Add overlays for the Pumpkin i350 to support THP7312 cameras. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- arch/arm64/boot/dts/mediatek/Makefile | 4 + .../mt8365-pumpkin-common-thp7312.dtsi | 23 ++++++ .../mt8365-pumpkin-csi0-thp7312-imx258.dtso | 73 +++++++++++++++++++ .../mt8365-pumpkin-csi1-thp7312-imx258.dtso | 73 +++++++++++++++++++ 4 files changed, 173 insertions(+) create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-common-thp7312.dtsi create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso create mode 100644 arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso