This new version is an offspring from the big "Allwinner A31/A83T
MIPI CSI-2 Support and A31 ISP Support" series, which was split into
individual series for better clarity and handling.
This part only concerns Allwinner platform support changes.
Note that the device-tree bindings for the MIPI CSI-2 controller
and ISP are still under review in their dedicated series, so these
patches should probably not be merged yet, although feedback about
them is welcome.
Changes since all-in-one v2:
- Corrected mbus index used for the interconnects;
- Used extended mbus binding and exported the DRAM clock for that;
- Reworked the description of the core openfirmware change to give
more insight about the situation.
Kévin L'hôpital (1):
ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865
Paul Kocialkowski (7):
of: Mark interconnects property supplier as optional
dt-bindings: interconnect: sunxi: Add V3s mbus compatible
clk: sunxi-ng: v3s: Export MBUS and DRAM clocks to the public header
ARM: dts: sun8i: v3s: Add mbus node to represent the interconnect
ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support
ARM: dts: sun8i: v3s: Add support for the ISP
ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node
.../arm/sunxi/allwinner,sun4i-a10-mbus.yaml | 2 +
arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 102 +++++++++++++++
arch/arm/boot/dts/sun8i-a83t.dtsi | 26 ++++
arch/arm/boot/dts/sun8i-v3s.dtsi | 121 ++++++++++++++++++
drivers/clk/sunxi-ng/ccu-sun8i-v3s.h | 4 -
drivers/of/property.c | 2 +-
include/dt-bindings/clock/sun8i-v3s-ccu.h | 4 +-
7 files changed, 254 insertions(+), 7 deletions(-)
--
2.35.1
In order to declare a mbus node for the v3s, expose its associated
clocks to the public header.
Signed-off-by: Paul Kocialkowski <[email protected]>
---
drivers/clk/sunxi-ng/ccu-sun8i-v3s.h | 4 ----
include/dt-bindings/clock/sun8i-v3s-ccu.h | 4 ++--
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.h b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.h
index 108eeeedcbf7..8ed4eff86ca1 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.h
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.h
@@ -39,14 +39,10 @@
/* The first bunch of module clocks are exported */
-#define CLK_DRAM 58
-
/* All the DRAM gates are exported */
/* Some more module clocks are exported */
-#define CLK_MBUS 72
-
/* And the GPU module clock is exported */
#define CLK_PLL_DDR1 74
diff --git a/include/dt-bindings/clock/sun8i-v3s-ccu.h b/include/dt-bindings/clock/sun8i-v3s-ccu.h
index 014ac6123d17..4231f23bc53b 100644
--- a/include/dt-bindings/clock/sun8i-v3s-ccu.h
+++ b/include/dt-bindings/clock/sun8i-v3s-ccu.h
@@ -87,7 +87,7 @@
#define CLK_SPI0 55
#define CLK_USB_PHY0 56
#define CLK_USB_OHCI0 57
-
+#define CLK_DRAM 58
#define CLK_DRAM_VE 59
#define CLK_DRAM_CSI 60
#define CLK_DRAM_EHCI 61
@@ -101,7 +101,7 @@
#define CLK_VE 69
#define CLK_AC_DIG 70
#define CLK_AVS 71
-
+#define CLK_MBUS 72
#define CLK_MIPI_CSI 73
/* Clocks not available on V3s */
--
2.35.1
The V3s uses the mbus interconnect to provide DRAM access for a
number of blocks. The SoC can only map 2 GiB of DRAM, which is
reflected in the dma-ranges property.
Signed-off-by: Paul Kocialkowski <[email protected]>
---
arch/arm/boot/dts/sun8i-v3s.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
index 084323d5c61c..a966b6a9acf5 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -577,6 +577,21 @@ int_mii_phy: ethernet-phy@1 {
};
};
+ mbus: dram-controller@1c62000 {
+ compatible = "allwinner,sun8i-v3s-mbus";
+ reg = <0x01c62000 0x1000>,
+ <0x01c63000 0x1000>;
+ reg-names = "mbus", "dram";
+ clocks = <&ccu CLK_MBUS>,
+ <&ccu CLK_DRAM>,
+ <&ccu CLK_BUS_DRAM>;
+ clock-names = "mbus", "dram", "bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges = <0x00000000 0x40000000 0x80000000>;
+ #interconnect-cells = <1>;
+ };
+
spi0: spi@1c68000 {
compatible = "allwinner,sun8i-h3-spi";
reg = <0x01c68000 0x1000>;
--
2.35.1
The V3s (and related platforms) come with an instance of the A31 ISP.
Even though it is very close to the A31 ISP, it is not exactly
register-compatible and a dedicated compatible only is used as a
result.
Just like most other blocks of the camera pipeline, the ISP uses
the common CSI bus, module and ram clock as well as reset.
A port connection to the ISP is added to CSI0 for convenience since
CSI0 serves for MIPI CSI-2 interface support, which is likely to
receive raw data that will need to be processed by the ISP to produce
a final image.
The interconnects property is used to inherit the proper DMA offset.
Signed-off-by: Paul Kocialkowski <[email protected]>
---
arch/arm/boot/dts/sun8i-v3s.dtsi | 35 ++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi
index a9405e011f3e..3d56a9471199 100644
--- a/arch/arm/boot/dts/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/sun8i-v3s.dtsi
@@ -643,6 +643,14 @@ csi0_in_mipi_csi2: endpoint {
remote-endpoint = <&mipi_csi2_out_csi0>;
};
};
+
+ port@2 {
+ reg = <2>;
+
+ csi0_out_isp: endpoint {
+ remote-endpoint = <&isp_in_csi0>;
+ };
+ };
};
};
@@ -701,5 +709,32 @@ csi1: camera@1cb4000 {
resets = <&ccu RST_BUS_CSI>;
status = "disabled";
};
+
+ isp: isp@1cb8000 {
+ compatible = "allwinner,sun8i-v3s-isp";
+ reg = <0x01cb8000 0x1000>;
+ interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_CSI>,
+ <&ccu CLK_CSI1_SCLK>,
+ <&ccu CLK_DRAM_CSI>;
+ clock-names = "bus", "mod", "ram";
+ resets = <&ccu RST_BUS_CSI>;
+ interconnects = <&mbus 5>;
+ interconnect-names = "dma-mem";
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ isp_in_csi0: endpoint {
+ remote-endpoint = <&csi0_out_isp>;
+ };
+ };
+ };
+ };
};
};
--
2.35.1
In order to set their correct DMA address offset, some devices rely on
the device-tree interconnects property which identifies an
interconnect node that provides a dma-ranges property that can be used
to set said offset.
Since that logic is all handled by the generic openfirmware and driver
code, the device-tree description could be enough to properly set
the offset.
However the interconnects property is currently not marked as
optional, which implies that a driver for the corresponding node
must be loaded as a requirement. When no such driver exists, this
results in an endless EPROBE_DEFER which gets propagated to the
calling driver. This ends up in the driver never loading.
Marking the interconnects property as optional makes it possible
to load the driver in that situation, since the EPROBE_DEFER return
code will no longer be propagated to the driver.
There might however be undesirable consequences with this change,
which I do not fully grasp at this point.
Signed-off-by: Paul Kocialkowski <[email protected]>
---
drivers/of/property.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 8e90071de6ed..ef7c56b510e8 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -1365,7 +1365,7 @@ static struct device_node *parse_interrupts(struct device_node *np,
static const struct supplier_bindings of_supplier_bindings[] = {
{ .parse_prop = parse_clocks, },
- { .parse_prop = parse_interconnects, },
+ { .parse_prop = parse_interconnects, .optional = true,},
{ .parse_prop = parse_iommus, .optional = true, },
{ .parse_prop = parse_iommu_maps, .optional = true, },
{ .parse_prop = parse_mboxes, },
--
2.35.1
From: Kévin L'hôpital <[email protected]>
The Bananapi M3 supports a camera module which includes an OV8865 sensor
connected via the parallel CSI interface and an OV8865 sensor connected
via MIPI CSI-2.
The I2C2 bus is shared by the two sensors as well as the (active-low)
reset signal, but each sensor has it own shutdown line.
Signed-off-by: Kévin L'hôpital <[email protected]>
Signed-off-by: Paul Kocialkowski <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 102 +++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 5a7e1bd5f825..80fd99cf24b2 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -85,6 +85,30 @@ led-1 {
};
};
+ reg_ov8865_avdd: ov8865-avdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov8865-avdd";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ vin-supply = <®_dldo4>;
+ };
+
+ reg_ov8865_dovdd: ov8865-dovdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov8865-dovdd";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ vin-supply = <®_dldo4>;
+ };
+
+ reg_ov8865_dvdd: ov8865-dvdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov8865-dvdd";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ vin-supply = <®_eldo1>;
+ };
+
reg_usb1_vbus: reg-usb1-vbus {
compatible = "regulator-fixed";
regulator-name = "usb1-vbus";
@@ -115,6 +139,23 @@ &cpu100 {
cpu-supply = <®_dcdc3>;
};
+&csi {
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ csi_in_mipi_csi2: endpoint {
+ remote-endpoint = <&mipi_csi2_out_csi>;
+ };
+ };
+ };
+};
+
&de {
status = "okay";
};
@@ -147,6 +188,36 @@ hdmi_out_con: endpoint {
};
};
+&i2c2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pe_pins>;
+ status = "okay";
+
+ ov8865: camera@36 {
+ compatible = "ovti,ov8865";
+ reg = <0x36>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&csi_mclk_pin>;
+ clocks = <&ccu CLK_CSI_MCLK>;
+ assigned-clocks = <&ccu CLK_CSI_MCLK>;
+ assigned-clock-rates = <24000000>;
+ avdd-supply = <®_ov8865_avdd>;
+ dovdd-supply = <®_ov8865_dovdd>;
+ dvdd-supply = <®_ov8865_dvdd>;
+ powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
+ reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+
+ port {
+ ov8865_out_mipi_csi2: endpoint {
+ data-lanes = <1 2 3 4>;
+ link-frequencies = /bits/ 64 <360000000>;
+
+ remote-endpoint = <&mipi_csi2_in_ov8865>;
+ };
+ };
+ };
+};
+
&mdio {
rgmii_phy: ethernet-phy@1 {
compatible = "ethernet-phy-ieee802.3-c22";
@@ -154,6 +225,24 @@ rgmii_phy: ethernet-phy@1 {
};
};
+&mipi_csi2 {
+ status = "okay";
+};
+
+&mipi_csi2_in {
+ mipi_csi2_in_ov8865: endpoint {
+ data-lanes = <1 2 3 4>;
+
+ remote-endpoint = <&ov8865_out_mipi_csi2>;
+ };
+};
+
+&mipi_csi2_out {
+ mipi_csi2_out_csi: endpoint {
+ remote-endpoint = <&csi_in_mipi_csi2>;
+ };
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins>;
@@ -327,11 +416,24 @@ ®_dldo3 {
regulator-name = "vcc-pd";
};
+®_dldo4 {
+ regulator-always-on;
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "avdd-csi";
+};
+
®_drivevbus {
regulator-name = "usb0-vbus";
status = "okay";
};
+®_eldo1 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "dvdd-csi-r";
+};
+
®_fldo1 {
regulator-min-microvolt = <1080000>;
regulator-max-microvolt = <1320000>;
--
2.35.1
+Saravana
On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> In order to set their correct DMA address offset, some devices rely on
> the device-tree interconnects property which identifies an
> interconnect node that provides a dma-ranges property that can be used
> to set said offset.
>
> Since that logic is all handled by the generic openfirmware and driver
> code, the device-tree description could be enough to properly set
> the offset.
>
> However the interconnects property is currently not marked as
> optional, which implies that a driver for the corresponding node
> must be loaded as a requirement. When no such driver exists, this
> results in an endless EPROBE_DEFER which gets propagated to the
> calling driver. This ends up in the driver never loading.
>
> Marking the interconnects property as optional makes it possible
> to load the driver in that situation, since the EPROBE_DEFER return
> code will no longer be propagated to the driver.
>
> There might however be undesirable consequences with this change,
> which I do not fully grasp at this point.
>
> Signed-off-by: Paul Kocialkowski <[email protected]>
> ---
> drivers/of/property.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 8e90071de6ed..ef7c56b510e8 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1365,7 +1365,7 @@ static struct device_node *parse_interrupts(struct device_node *np,
>
> static const struct supplier_bindings of_supplier_bindings[] = {
> { .parse_prop = parse_clocks, },
> - { .parse_prop = parse_interconnects, },
> + { .parse_prop = parse_interconnects, .optional = true,},
> { .parse_prop = parse_iommus, .optional = true, },
> { .parse_prop = parse_iommu_maps, .optional = true, },
> { .parse_prop = parse_mboxes, },
> --
> 2.35.1
>
>
On Mon, Mar 7, 2022 at 3:21 PM Rob Herring <[email protected]> wrote:
>
> +Saravana
>
> On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> > In order to set their correct DMA address offset, some devices rely on
> > the device-tree interconnects property which identifies an
> > interconnect node that provides a dma-ranges property that can be used
> > to set said offset.
> >
> > Since that logic is all handled by the generic openfirmware and driver
> > code, the device-tree description could be enough to properly set
> > the offset.
> >
> > However the interconnects property is currently not marked as
> > optional, which implies that a driver for the corresponding node
> > must be loaded as a requirement. When no such driver exists, this
> > results in an endless EPROBE_DEFER which gets propagated to the
> > calling driver. This ends up in the driver never loading.
> >
> > Marking the interconnects property as optional makes it possible
> > to load the driver in that situation, since the EPROBE_DEFER return
> > code will no longer be propagated to the driver.
> >
> > There might however be undesirable consequences with this change,
> > which I do not fully grasp at this point.
Temporary NACK till I get a bit more time to take a closer look. I
really don't like the idea of making interconnects optional. IOMMUs
and DMAs were exceptions. Also, we kinda discuss similar issues in
LPC. We had some consensus on how to handle these and I noted them all
down with a lot of details -- let me go take a look at those notes
again and see if I can send a more generic patch.
Paul,
Can you point to the DTS (not DTSI) file that corresponds to this?
Also, if it's a builtin kernel, I'd recommend setting
deferred_probe_timeout=1 and that should take care of it too.
-Saravana
> >
> > Signed-off-by: Paul Kocialkowski <[email protected]>
> > ---
> > drivers/of/property.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/of/property.c b/drivers/of/property.c
> > index 8e90071de6ed..ef7c56b510e8 100644
> > --- a/drivers/of/property.c
> > +++ b/drivers/of/property.c
> > @@ -1365,7 +1365,7 @@ static struct device_node *parse_interrupts(struct device_node *np,
> >
> > static const struct supplier_bindings of_supplier_bindings[] = {
> > { .parse_prop = parse_clocks, },
> > - { .parse_prop = parse_interconnects, },
> > + { .parse_prop = parse_interconnects, .optional = true,},
> > { .parse_prop = parse_iommus, .optional = true, },
> > { .parse_prop = parse_iommu_maps, .optional = true, },
> > { .parse_prop = parse_mboxes, },
> > --
> > 2.35.1
> >
> >
On 3/2/22 3:10 PM, Paul Kocialkowski wrote:
> The V3s uses the mbus interconnect to provide DRAM access for a
> number of blocks. The SoC can only map 2 GiB of DRAM, which is
> reflected in the dma-ranges property.
>
> Signed-off-by: Paul Kocialkowski <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
On 3/2/22 3:10 PM, Paul Kocialkowski wrote:
> In order to declare a mbus node for the v3s, expose its associated
> clocks to the public header.
>
> Signed-off-by: Paul Kocialkowski <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Hi,
On Mon, Mar 07, 2022 at 07:34:22PM -0800, Saravana Kannan wrote:
> On Mon, Mar 7, 2022 at 3:21 PM Rob Herring <[email protected]> wrote:
> >
> > +Saravana
> >
> > On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> > > In order to set their correct DMA address offset, some devices rely on
> > > the device-tree interconnects property which identifies an
> > > interconnect node that provides a dma-ranges property that can be used
> > > to set said offset.
> > >
> > > Since that logic is all handled by the generic openfirmware and driver
> > > code, the device-tree description could be enough to properly set
> > > the offset.
> > >
> > > However the interconnects property is currently not marked as
> > > optional, which implies that a driver for the corresponding node
> > > must be loaded as a requirement. When no such driver exists, this
> > > results in an endless EPROBE_DEFER which gets propagated to the
> > > calling driver. This ends up in the driver never loading.
> > >
> > > Marking the interconnects property as optional makes it possible
> > > to load the driver in that situation, since the EPROBE_DEFER return
> > > code will no longer be propagated to the driver.
> > >
> > > There might however be undesirable consequences with this change,
> > > which I do not fully grasp at this point.
>
> Temporary NACK till I get a bit more time to take a closer look. I
> really don't like the idea of making interconnects optional. IOMMUs
> and DMAs were exceptions. Also, we kinda discuss similar issues in
> LPC. We had some consensus on how to handle these and I noted them all
> down with a lot of details -- let me go take a look at those notes
> again and see if I can send a more generic patch.
>
> Paul,
>
> Can you point to the DTS (not DTSI) file that corresponds to this?
> Also, if it's a builtin kernel, I'd recommend setting
> deferred_probe_timeout=1 and that should take care of it too.
For the record, I also encountered this today on next-20220726 with this
device:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun5i.dtsi#n775
The driver won't probe without fw_devlink=off
Maxime
Hi,
On Wed 27 Jul 22, 09:06, Saravana Kannan wrote:
> On Wed, Jul 27, 2022 at 5:06 AM Maxime Ripard <[email protected]> wrote:
> >
> > Hi,
> >
> > On Mon, Mar 07, 2022 at 07:34:22PM -0800, Saravana Kannan wrote:
> > > On Mon, Mar 7, 2022 at 3:21 PM Rob Herring <[email protected]> wrote:
> > > >
> > > > +Saravana
> > > >
> > > > On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> > > > > In order to set their correct DMA address offset, some devices rely on
> > > > > the device-tree interconnects property which identifies an
> > > > > interconnect node that provides a dma-ranges property that can be used
> > > > > to set said offset.
> > > > >
> > > > > Since that logic is all handled by the generic openfirmware and driver
> > > > > code, the device-tree description could be enough to properly set
> > > > > the offset.
> > > > >
> > > > > However the interconnects property is currently not marked as
> > > > > optional, which implies that a driver for the corresponding node
> > > > > must be loaded as a requirement. When no such driver exists, this
> > > > > results in an endless EPROBE_DEFER which gets propagated to the
> > > > > calling driver. This ends up in the driver never loading.
> > > > >
> > > > > Marking the interconnects property as optional makes it possible
> > > > > to load the driver in that situation, since the EPROBE_DEFER return
> > > > > code will no longer be propagated to the driver.
> > > > >
> > > > > There might however be undesirable consequences with this change,
> > > > > which I do not fully grasp at this point.
> > >
> > > Temporary NACK till I get a bit more time to take a closer look. I
> > > really don't like the idea of making interconnects optional. IOMMUs
> > > and DMAs were exceptions. Also, we kinda discuss similar issues in
> > > LPC. We had some consensus on how to handle these and I noted them all
> > > down with a lot of details -- let me go take a look at those notes
> > > again and see if I can send a more generic patch.
> > >
> > > Paul,
> > >
> > > Can you point to the DTS (not DTSI) file that corresponds to this?
> > > Also, if it's a builtin kernel, I'd recommend setting
> > > deferred_probe_timeout=1 and that should take care of it too.
> >
> > For the record, I also encountered this today on next-20220726 with this
> > device:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun5i.dtsi#n775
> >
> > The driver won't probe without fw_devlink=off
>
> Really? I basically ended up doing what I mentioned in my original
> reply. next-20220726 should have my changes that'll make sure
> fw_devlink doesn't block any probe (it'll still try to create as many
> device links as possible) after 10s (default deferred probe timeout).
> Can you try to find more info on why it's not probing?
> <debugfs>/devices_deferred should give more details.
By the way last time I checked the initial issue that I reported appeared to be
fixed by the patch (Extend deferred probe timeout on driver registration).
Cheers,
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
On Wed, Jul 27, 2022 at 5:06 AM Maxime Ripard <[email protected]> wrote:
>
> Hi,
>
> On Mon, Mar 07, 2022 at 07:34:22PM -0800, Saravana Kannan wrote:
> > On Mon, Mar 7, 2022 at 3:21 PM Rob Herring <[email protected]> wrote:
> > >
> > > +Saravana
> > >
> > > On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> > > > In order to set their correct DMA address offset, some devices rely on
> > > > the device-tree interconnects property which identifies an
> > > > interconnect node that provides a dma-ranges property that can be used
> > > > to set said offset.
> > > >
> > > > Since that logic is all handled by the generic openfirmware and driver
> > > > code, the device-tree description could be enough to properly set
> > > > the offset.
> > > >
> > > > However the interconnects property is currently not marked as
> > > > optional, which implies that a driver for the corresponding node
> > > > must be loaded as a requirement. When no such driver exists, this
> > > > results in an endless EPROBE_DEFER which gets propagated to the
> > > > calling driver. This ends up in the driver never loading.
> > > >
> > > > Marking the interconnects property as optional makes it possible
> > > > to load the driver in that situation, since the EPROBE_DEFER return
> > > > code will no longer be propagated to the driver.
> > > >
> > > > There might however be undesirable consequences with this change,
> > > > which I do not fully grasp at this point.
> >
> > Temporary NACK till I get a bit more time to take a closer look. I
> > really don't like the idea of making interconnects optional. IOMMUs
> > and DMAs were exceptions. Also, we kinda discuss similar issues in
> > LPC. We had some consensus on how to handle these and I noted them all
> > down with a lot of details -- let me go take a look at those notes
> > again and see if I can send a more generic patch.
> >
> > Paul,
> >
> > Can you point to the DTS (not DTSI) file that corresponds to this?
> > Also, if it's a builtin kernel, I'd recommend setting
> > deferred_probe_timeout=1 and that should take care of it too.
>
> For the record, I also encountered this today on next-20220726 with this
> device:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun5i.dtsi#n775
>
> The driver won't probe without fw_devlink=off
Really? I basically ended up doing what I mentioned in my original
reply. next-20220726 should have my changes that'll make sure
fw_devlink doesn't block any probe (it'll still try to create as many
device links as possible) after 10s (default deferred probe timeout).
Can you try to find more info on why it's not probing?
<debugfs>/devices_deferred should give more details.
-Saravana
On Wed, Jul 27, 2022 at 10:17 AM Paul Kocialkowski
<[email protected]> wrote:
>
> Hi,
>
> On Wed 27 Jul 22, 09:06, Saravana Kannan wrote:
> > On Wed, Jul 27, 2022 at 5:06 AM Maxime Ripard <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Mar 07, 2022 at 07:34:22PM -0800, Saravana Kannan wrote:
> > > > On Mon, Mar 7, 2022 at 3:21 PM Rob Herring <[email protected]> wrote:
> > > > >
> > > > > +Saravana
> > > > >
> > > > > On Wed, Mar 02, 2022 at 10:10:53PM +0100, Paul Kocialkowski wrote:
> > > > > > In order to set their correct DMA address offset, some devices rely on
> > > > > > the device-tree interconnects property which identifies an
> > > > > > interconnect node that provides a dma-ranges property that can be used
> > > > > > to set said offset.
> > > > > >
> > > > > > Since that logic is all handled by the generic openfirmware and driver
> > > > > > code, the device-tree description could be enough to properly set
> > > > > > the offset.
> > > > > >
> > > > > > However the interconnects property is currently not marked as
> > > > > > optional, which implies that a driver for the corresponding node
> > > > > > must be loaded as a requirement. When no such driver exists, this
> > > > > > results in an endless EPROBE_DEFER which gets propagated to the
> > > > > > calling driver. This ends up in the driver never loading.
> > > > > >
> > > > > > Marking the interconnects property as optional makes it possible
> > > > > > to load the driver in that situation, since the EPROBE_DEFER return
> > > > > > code will no longer be propagated to the driver.
> > > > > >
> > > > > > There might however be undesirable consequences with this change,
> > > > > > which I do not fully grasp at this point.
> > > >
> > > > Temporary NACK till I get a bit more time to take a closer look. I
> > > > really don't like the idea of making interconnects optional. IOMMUs
> > > > and DMAs were exceptions. Also, we kinda discuss similar issues in
> > > > LPC. We had some consensus on how to handle these and I noted them all
> > > > down with a lot of details -- let me go take a look at those notes
> > > > again and see if I can send a more generic patch.
> > > >
> > > > Paul,
> > > >
> > > > Can you point to the DTS (not DTSI) file that corresponds to this?
> > > > Also, if it's a builtin kernel, I'd recommend setting
> > > > deferred_probe_timeout=1 and that should take care of it too.
> > >
> > > For the record, I also encountered this today on next-20220726 with this
> > > device:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun5i.dtsi#n775
> > >
> > > The driver won't probe without fw_devlink=off
> >
> > Really? I basically ended up doing what I mentioned in my original
> > reply. next-20220726 should have my changes that'll make sure
> > fw_devlink doesn't block any probe (it'll still try to create as many
> > device links as possible) after 10s (default deferred probe timeout).
> > Can you try to find more info on why it's not probing?
> > <debugfs>/devices_deferred should give more details.
>
> By the way last time I checked the initial issue that I reported appeared to be
> fixed by the patch (Extend deferred probe timeout on driver registration).
Thanks for the confirmation.
-Saravana