linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: dts: imx6qdl: tqma6: fix indentation
@ 2020-08-24  9:10 Matthias Schiffer
  2020-08-24  9:10 ` [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-24  9:10 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer
  Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, linux-kernel, Matthias Schiffer

Fixes: cac849e9bbc8 ("ARM: dts: imx6qdl: add TQMa6{S,Q,QP} SoM")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 arch/arm/boot/dts/imx6qdl-tqma6.dtsi | 188 +++++++++++++--------------
 1 file changed, 94 insertions(+), 94 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
index 29bcce20f5f3..9513020ddd1a 100644
--- a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
@@ -83,101 +83,101 @@
 };
 
 &pmic {
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_pmic>;
-		interrupt-parent = <&gpio6>;
-		interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
-
-		regulators {
-			reg_vddcore: sw1ab {
-				regulator-min-microvolt = <300000>;
-				regulator-max-microvolt = <1875000>;
-				regulator-always-on;
-			};
-
-			reg_vddsoc: sw1c {
-				regulator-min-microvolt = <300000>;
-				regulator-max-microvolt = <1875000>;
-				regulator-always-on;
-			};
-
-			reg_gen_3v3: sw2 {
-				regulator-min-microvolt = <800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			reg_ddr_1v5a: sw3a {
-				regulator-min-microvolt = <400000>;
-				regulator-max-microvolt = <1975000>;
-				regulator-always-on;
-			};
-
-			reg_ddr_1v5b: sw3b {
-				regulator-min-microvolt = <400000>;
-				regulator-max-microvolt = <1975000>;
-				regulator-always-on;
-			};
-
-			sw4_reg: sw4 {
-				regulator-min-microvolt = <800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			reg_5v_600mA: swbst {
-				regulator-min-microvolt = <5000000>;
-				regulator-max-microvolt = <5150000>;
-				regulator-always-on;
-			};
-
-			reg_snvs_3v: vsnvs {
-				regulator-min-microvolt = <1500000>;
-				regulator-max-microvolt = <3000000>;
-				regulator-always-on;
-			};
-
-			reg_vrefddr: vrefddr {
-				regulator-boot-on;
-				regulator-always-on;
-			};
-
-			reg_vgen1_1v5: vgen1 {
-				regulator-min-microvolt = <800000>;
-				regulator-max-microvolt = <1550000>;
-				/* not used */
-			};
-
-			reg_vgen2_1v2_eth: vgen2 {
-				regulator-min-microvolt = <800000>;
-				regulator-max-microvolt = <1550000>;
-				regulator-always-on;
-			};
-
-			reg_vgen3_2v8: vgen3 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			reg_vgen4_1v8: vgen4 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			reg_vgen5_1v8_eth: vgen5 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
-
-			reg_vgen6_3v3: vgen6 {
-				regulator-min-microvolt = <1800000>;
-				regulator-max-microvolt = <3300000>;
-				regulator-always-on;
-			};
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pmic>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
+
+	regulators {
+		reg_vddcore: sw1ab {
+			regulator-min-microvolt = <300000>;
+			regulator-max-microvolt = <1875000>;
+			regulator-always-on;
+		};
+
+		reg_vddsoc: sw1c {
+			regulator-min-microvolt = <300000>;
+			regulator-max-microvolt = <1875000>;
+			regulator-always-on;
+		};
+
+		reg_gen_3v3: sw2 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		reg_ddr_1v5a: sw3a {
+			regulator-min-microvolt = <400000>;
+			regulator-max-microvolt = <1975000>;
+			regulator-always-on;
+		};
+
+		reg_ddr_1v5b: sw3b {
+			regulator-min-microvolt = <400000>;
+			regulator-max-microvolt = <1975000>;
+			regulator-always-on;
+		};
+
+		sw4_reg: sw4 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		reg_5v_600mA: swbst {
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5150000>;
+			regulator-always-on;
+		};
+
+		reg_snvs_3v: vsnvs {
+			regulator-min-microvolt = <1500000>;
+			regulator-max-microvolt = <3000000>;
+			regulator-always-on;
 		};
+
+		reg_vrefddr: vrefddr {
+			regulator-boot-on;
+			regulator-always-on;
+		};
+
+		reg_vgen1_1v5: vgen1 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <1550000>;
+			/* not used */
+		};
+
+		reg_vgen2_1v2_eth: vgen2 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <1550000>;
+			regulator-always-on;
+		};
+
+		reg_vgen3_2v8: vgen3 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		reg_vgen4_1v8: vgen4 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		reg_vgen5_1v8_eth: vgen5 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+
+		reg_vgen6_3v3: vgen6 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+	};
 };
 
 /* eMMC */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-24  9:10 [PATCH 1/2] ARM: dts: imx6qdl: tqma6: fix indentation Matthias Schiffer
@ 2020-08-24  9:10 ` Matthias Schiffer
  2020-08-24 21:36   ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-24  9:10 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer
  Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, linux-kernel, Matthias Schiffer

- Fix national,lm75 compatible string
- Replace obsolete fsl,spi-num-chipselects with num-cs

Fixes: cac849e9bbc8 ("ARM: dts: imx6qdl: add TQMa6{S,Q,QP} SoM")
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 arch/arm/boot/dts/imx6qdl-tqma6.dtsi  | 2 +-
 arch/arm/boot/dts/imx6qdl-tqma6a.dtsi | 2 +-
 arch/arm/boot/dts/imx6qdl-tqma6b.dtsi | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
index 9513020ddd1a..7aaae83c1fae 100644
--- a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
@@ -20,7 +20,7 @@
 &ecspi1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1>;
-	fsl,spi-num-chipselects = <1>;
+	num-cs = <1>;
 	cs-gpios = <&gpio3 19 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx6qdl-tqma6a.dtsi b/arch/arm/boot/dts/imx6qdl-tqma6a.dtsi
index c18a06cf7929..b679bec78e6c 100644
--- a/arch/arm/boot/dts/imx6qdl-tqma6a.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tqma6a.dtsi
@@ -16,7 +16,7 @@
 	};
 
 	sensor@48 {
-		compatible = "lm75";
+		compatible = "national,lm75";
 		reg = <0x48>;
 	};
 
diff --git a/arch/arm/boot/dts/imx6qdl-tqma6b.dtsi b/arch/arm/boot/dts/imx6qdl-tqma6b.dtsi
index a7460075f517..49c472285c06 100644
--- a/arch/arm/boot/dts/imx6qdl-tqma6b.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-tqma6b.dtsi
@@ -16,7 +16,7 @@
 	};
 
 	sensor@48 {
-		compatible = "lm75";
+		compatible = "national,lm75";
 		reg = <0x48>;
 	};
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-24  9:10 ` [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes Matthias Schiffer
@ 2020-08-24 21:36   ` Fabio Estevam
  2020-08-25  7:22     ` (EXT) " Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-24 21:36 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Matthias,

On Mon, Aug 24, 2020 at 6:10 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> diff --git a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> index 9513020ddd1a..7aaae83c1fae 100644
> --- a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> @@ -20,7 +20,7 @@
>  &ecspi1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_ecspi1>;
> -       fsl,spi-num-chipselects = <1>;
> +       num-cs = <1>;

You could simply remove fsl,spi-num-chipselects without passing num-cs.

The spi core is able to count the number of chipselects passed via
cs-gpios in the device tree.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-24 21:36   ` Fabio Estevam
@ 2020-08-25  7:22     ` Matthias Schiffer
  2020-08-25 14:24       ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-25  7:22 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Mon, 2020-08-24 at 18:36 -0300, Fabio Estevam wrote:
> Hi Matthias,
> 
> On Mon, Aug 24, 2020 at 6:10 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > diff --git a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> > b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> > index 9513020ddd1a..7aaae83c1fae 100644
> > --- a/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-tqma6.dtsi
> > @@ -20,7 +20,7 @@
> >  &ecspi1 {
> >         pinctrl-names = "default";
> >         pinctrl-0 = <&pinctrl_ecspi1>;
> > -       fsl,spi-num-chipselects = <1>;
> > +       num-cs = <1>;
> 
> You could simply remove fsl,spi-num-chipselects without passing num-
> cs.
> 
> The spi core is able to count the number of chipselects passed via
> cs-gpios in the device tree.

Hmm, unless I'm overlooking something, this is not going to work:

- spi_get_gpio_descs() sets num_chipselect to the maximum of the
num_chipselect set in the driver and the number of cs-gpios

- spi_imx_probe() sets num_chipselect to 3 if not specified in the
device tree

So I think we would end up with 3 instead of 1 chipselect.


Kind regards,
Matthias


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-25  7:22     ` (EXT) " Matthias Schiffer
@ 2020-08-25 14:24       ` Fabio Estevam
  2020-08-25 14:40         ` (EXT) " Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-25 14:24 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Matthias,

On Tue, Aug 25, 2020 at 4:22 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> Hmm, unless I'm overlooking something, this is not going to work:
>
> - spi_get_gpio_descs() sets num_chipselect to the maximum of the
> num_chipselect set in the driver and the number of cs-gpios
>
> - spi_imx_probe() sets num_chipselect to 3 if not specified in the
> device tree
>
> So I think we would end up with 3 instead of 1 chipselect.

Oh, this has changed recently in 8cdcd8aeee281 ("spi: imx/fsl-lpspi:
Convert to GPIO descriptors"):
....

-       } else {
-               u32 num_cs;
-
-               if (!of_property_read_u32(np, "num-cs", &num_cs))
-                       master->num_chipselect = num_cs;
-               /* If not preset, default value of 1 is used */

Initially, if num-cs was not present the default value for num_chipselect was 1.

-       }
+       /*
+        * Get number of chip selects from device properties. This can be
+        * coming from device tree or boardfiles, if it is not defined,
+        * a default value of 3 chip selects will be used, as all the legacy
+        * board files have <= 3 chip selects.
+        */
+       if (!device_property_read_u32(&pdev->dev, "num-cs", &val))
+               master->num_chipselect = val;
+       else
+               master->num_chipselect = 3;

Now it became 3.

I think this is a driver issue and we should fix the driver instead of
requiring to pass num-cs to the device tree.


num-cs is not even documented in the spi-imx binding.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-25 14:24       ` Fabio Estevam
@ 2020-08-25 14:40         ` Matthias Schiffer
  2020-08-25 17:16           ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-25 14:40 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Tue, 2020-08-25 at 11:24 -0300, Fabio Estevam wrote:
> Hi Matthias,
> 
> On Tue, Aug 25, 2020 at 4:22 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > Hmm, unless I'm overlooking something, this is not going to work:
> > 
> > - spi_get_gpio_descs() sets num_chipselect to the maximum of the
> > num_chipselect set in the driver and the number of cs-gpios
> > 
> > - spi_imx_probe() sets num_chipselect to 3 if not specified in the
> > device tree
> > 
> > So I think we would end up with 3 instead of 1 chipselect.
> 
> Oh, this has changed recently in 8cdcd8aeee281 ("spi: imx/fsl-lpspi:
> Convert to GPIO descriptors"):
> ....
> 
> -       } else {
> -               u32 num_cs;
> -
> -               if (!of_property_read_u32(np, "num-cs", &num_cs))
> -                       master->num_chipselect = num_cs;
> -               /* If not preset, default value of 1 is used */
> 
> Initially, if num-cs was not present the default value for
> num_chipselect was 1.
> 
> -       }
> +       /*
> +        * Get number of chip selects from device properties. This
> can be
> +        * coming from device tree or boardfiles, if it is not
> defined,
> +        * a default value of 3 chip selects will be used, as all the
> legacy
> +        * board files have <= 3 chip selects.
> +        */
> +       if (!device_property_read_u32(&pdev->dev, "num-cs", &val))
> +               master->num_chipselect = val;
> +       else
> +               master->num_chipselect = 3;
> 
> Now it became 3.
> 
> I think this is a driver issue and we should fix the driver instead
> of
> requiring to pass num-cs to the device tree.
> 
> 
> num-cs is not even documented in the spi-imx binding.

Makes sense. Does the following logic sound correct?

- If num-cs is set, use that (and add it to the docs)
- If num-cs is unset, use the number of cs-gpios
- If num-cs is unset and no cs-gpios are defined, use a driver-provided 
default


I'm not sure if 3 is a particularly useful default either, but it seems
it was chosen to accommodate boards that previously set this via
platform data. All SoCs I've checked (i.MX6Q/DL, i.MX6UL, i.MX7) have 4
internal CS pins per ECSPI instance, so maybe the driver should use
that as its default instead?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-25 14:40         ` (EXT) " Matthias Schiffer
@ 2020-08-25 17:16           ` Fabio Estevam
  2020-08-26 10:32             ` (EXT) " Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-25 17:16 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Tue, Aug 25, 2020 at 11:40 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> Makes sense. Does the following logic sound correct?
>
> - If num-cs is set, use that (and add it to the docs)

I would not add num-cs to the docs. As far as I can see there is no
imx dts that uses num-cs currently.

> - If num-cs is unset, use the number of cs-gpios
> - If num-cs is unset and no cs-gpios are defined, use a driver-provided
> default
>
>
> I'm not sure if 3 is a particularly useful default either, but it seems
> it was chosen to accommodate boards that previously set this via
> platform data. All SoCs I've checked (i.MX6Q/DL, i.MX6UL, i.MX7) have 4
> internal CS pins per ECSPI instance, so maybe the driver should use
> that as its default instead?

I think it is time to get rid of i.MX board files. I will try to work
on this when I have a chance.

bout using 4 as default chip select number, please also check some
older SoCs like imx25, imx35, imx51, imx53, etc

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-25 17:16           ` Fabio Estevam
@ 2020-08-26 10:32             ` Matthias Schiffer
  2020-08-26 10:34               ` Matthias Schiffer
  2020-08-26 10:59               ` Fabio Estevam
  0 siblings, 2 replies; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-26 10:32 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Tue, 2020-08-25 at 14:16 -0300, Fabio Estevam wrote:
> On Tue, Aug 25, 2020 at 11:40 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > Makes sense. Does the following logic sound correct?
> > 
> > - If num-cs is set, use that (and add it to the docs)
> 
> I would not add num-cs to the docs. As far as I can see there is no
> imx dts that uses num-cs currently.

But the previous platform data that was removed in 8cdcd8aeee281 ("spi:
imx/fsl-lpspi: Convert to GPIO descriptors") set different values for
different boards. So maybe some DTS should be using num-cs?


> 
> > - If num-cs is unset, use the number of cs-gpios
> > - If num-cs is unset and no cs-gpios are defined, use a driver-
> > provided
> > default
> > 
> > 
> > I'm not sure if 3 is a particularly useful default either, but it
> > seems
> > it was chosen to accommodate boards that previously set this via
> > platform data. All SoCs I've checked (i.MX6Q/DL, i.MX6UL, i.MX7)
> > have 4
> > internal CS pins per ECSPI instance, so maybe the driver should use
> > that as its default instead?
> 
> I think it is time to get rid of i.MX board files. I will try to work
> on this when I have a chance.
> 
> bout using 4 as default chip select number, please also check some
> older SoCs like imx25, imx35, imx51, imx53, etc

Hmm, I just checked i.MX28, and it has only 3 chip selects per
instance.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 10:32             ` (EXT) " Matthias Schiffer
@ 2020-08-26 10:34               ` Matthias Schiffer
  2020-08-26 10:59               ` Fabio Estevam
  1 sibling, 0 replies; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-26 10:34 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, 2020-08-26 at 12:32 +0200, Matthias Schiffer wrote:
> On Tue, 2020-08-25 at 14:16 -0300, Fabio Estevam wrote:
> > On Tue, Aug 25, 2020 at 11:40 AM Matthias Schiffer
> > <matthias.schiffer@ew.tq-group.com> wrote:
> > 
> > > Makes sense. Does the following logic sound correct?
> > > 
> > > - If num-cs is set, use that (and add it to the docs)
> > 
> > I would not add num-cs to the docs. As far as I can see there is no
> > imx dts that uses num-cs currently.
> 
> But the previous platform data that was removed in 8cdcd8aeee281
> ("spi:
> imx/fsl-lpspi: Convert to GPIO descriptors") set different values for
> different boards. So maybe some DTS should be using num-cs?
> 
> 
> > 
> > > - If num-cs is unset, use the number of cs-gpios
> > > - If num-cs is unset and no cs-gpios are defined, use a driver-
> > > provided
> > > default
> > > 
> > > 
> > > I'm not sure if 3 is a particularly useful default either, but it
> > > seems
> > > it was chosen to accommodate boards that previously set this via
> > > platform data. All SoCs I've checked (i.MX6Q/DL, i.MX6UL, i.MX7)
> > > have 4
> > > internal CS pins per ECSPI instance, so maybe the driver should
> > > use
> > > that as its default instead?
> > 
> > I think it is time to get rid of i.MX board files. I will try to
> > work
> > on this when I have a chance.
> > 
> > bout using 4 as default chip select number, please also check some
> > older SoCs like imx25, imx35, imx51, imx53, etc
> 
> Hmm, I just checked i.MX28, and it has only 3 chip selects per
> instance.

Ah sorry, I got confused, the i.MX28 has a different SPI IP.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 10:32             ` (EXT) " Matthias Schiffer
  2020-08-26 10:34               ` Matthias Schiffer
@ 2020-08-26 10:59               ` Fabio Estevam
  2020-08-26 11:54                 ` (EXT) " Matthias Schiffer
  1 sibling, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-26 10:59 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Matthias,

On Wed, Aug 26, 2020 at 7:32 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> But the previous platform data that was removed in 8cdcd8aeee281 ("spi:
> imx/fsl-lpspi: Convert to GPIO descriptors") set different values for
> different boards. So maybe some DTS should be using num-cs?

Could you provide an example of an imx dts that should use num-cs?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 10:59               ` Fabio Estevam
@ 2020-08-26 11:54                 ` Matthias Schiffer
  2020-08-26 13:01                   ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-26 11:54 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, 2020-08-26 at 07:59 -0300, Fabio Estevam wrote:
> Hi Matthias,
> 
> On Wed, Aug 26, 2020 at 7:32 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > But the previous platform data that was removed in 8cdcd8aeee281
> > ("spi:
> > imx/fsl-lpspi: Convert to GPIO descriptors") set different values
> > for
> > different boards. So maybe some DTS should be using num-cs?
> 
> Could you provide an example of an imx dts that should use num-cs?

I'm not sure, does anything break when num_chipselect is set too high?

Before 8cdcd8aeee281, num_chipselect was set to 3 for spi0 and to 1 for
spi1 in arch/arm/mach-imx/mach-mx31lite.c. My understanding is that it
would make sense to add this as num-cs to
arch/arm/boot/dts/imx31-lite.dts.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 11:54                 ` (EXT) " Matthias Schiffer
@ 2020-08-26 13:01                   ` Fabio Estevam
  2020-08-26 13:13                     ` (EXT) " Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-26 13:01 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, Aug 26, 2020 at 8:54 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> Before 8cdcd8aeee281, num_chipselect was set to 3 for spi0 and to 1 for
> spi1 in arch/arm/mach-imx/mach-mx31lite.c. My understanding is that it
> would make sense to add this as num-cs to
> arch/arm/boot/dts/imx31-lite.dts.

Or just pass cs-gpios instead?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 13:01                   ` Fabio Estevam
@ 2020-08-26 13:13                     ` Matthias Schiffer
  2020-08-26 13:49                       ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-26 13:13 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, 2020-08-26 at 10:01 -0300, Fabio Estevam wrote:
> On Wed, Aug 26, 2020 at 8:54 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > Before 8cdcd8aeee281, num_chipselect was set to 3 for spi0 and to 1
> > for
> > spi1 in arch/arm/mach-imx/mach-mx31lite.c. My understanding is that
> > it
> > would make sense to add this as num-cs to
> > arch/arm/boot/dts/imx31-lite.dts.
> 
> Or just pass cs-gpios instead?

Using GPIOs for chipselect would require different pinmuxing. Also, why
use GPIOs, when the SPI controller has this built in?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: (EXT) Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 13:13                     ` (EXT) " Matthias Schiffer
@ 2020-08-26 13:49                       ` Fabio Estevam
  2020-08-27  7:31                         ` Matthias Schiffer
  0 siblings, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2020-08-26 13:49 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, Aug 26, 2020 at 10:13 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> Using GPIOs for chipselect would require different pinmuxing. Also, why
> use GPIOs, when the SPI controller has this built in?

In the initial chips with the ECSPI controller there was a bug with
the native chipselect controller and we had to use GPIO.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-26 13:49                       ` Fabio Estevam
@ 2020-08-27  7:31                         ` Matthias Schiffer
  2020-08-27 21:27                           ` Fabio Estevam
  0 siblings, 1 reply; 16+ messages in thread
From: Matthias Schiffer @ 2020-08-27  7:31 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

On Wed, 2020-08-26 at 10:49 -0300, Fabio Estevam wrote:
> On Wed, Aug 26, 2020 at 10:13 AM Matthias Schiffer
> <matthias.schiffer@ew.tq-group.com> wrote:
> 
> > Using GPIOs for chipselect would require different pinmuxing. Also,
> > why
> > use GPIOs, when the SPI controller has this built in?
> 
> In the initial chips with the ECSPI controller there was a bug with
> the native chipselect controller and we had to use GPIO.

Ah, I see.

Nevertheless, hardware that uses the native chipselects of newer chips
exists (for example our TQ-Systems starterkit mainboards, the DTS of
which we're currently preparing for mainline submission). Shouldn't
num-cs be set for boards (or SoM DTSI) where not all CS pins of the SoC
are usable?

In any case, my original question was about the intended logic for
num_chipselects for SPI drivers. My proposal was:

- If num-cs is set, use that
- If num-cs is unset, use the number of cs-gpios
- If num-cs is unset and no cs-gpios are defined, use a driver-provided 
default

How do other SPI controller drivers deal with this?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes
  2020-08-27  7:31                         ` Matthias Schiffer
@ 2020-08-27 21:27                           ` Fabio Estevam
  0 siblings, 0 replies; 16+ messages in thread
From: Fabio Estevam @ 2020-08-27 21:27 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel

Hi Matthias,

(Your mailer is messing the threading.)

On Thu, Aug 27, 2020 at 4:31 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:

> Ah, I see.
>
> Nevertheless, hardware that uses the native chipselects of newer chips
> exists (for example our TQ-Systems starterkit mainboards, the DTS of
> which we're currently preparing for mainline submission). Shouldn't
> num-cs be set for boards (or SoM DTSI) where not all CS pins of the SoC
> are usable?
>
> In any case, my original question was about the intended logic for
> num_chipselects for SPI drivers. My proposal was:
>
> - If num-cs is set, use that
> - If num-cs is unset, use the number of cs-gpios
> - If num-cs is unset and no cs-gpios are defined, use a driver-provided
> default
>
> How do other SPI controller drivers deal with this?

I think it would be better to discuss this in the spi mailing list
with Mark Brown and the dt maintainer, Rob Herring.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-08-27 21:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24  9:10 [PATCH 1/2] ARM: dts: imx6qdl: tqma6: fix indentation Matthias Schiffer
2020-08-24  9:10 ` [PATCH 2/2] ARM: dts: imx6qdl: tqma6: minor fixes Matthias Schiffer
2020-08-24 21:36   ` Fabio Estevam
2020-08-25  7:22     ` (EXT) " Matthias Schiffer
2020-08-25 14:24       ` Fabio Estevam
2020-08-25 14:40         ` (EXT) " Matthias Schiffer
2020-08-25 17:16           ` Fabio Estevam
2020-08-26 10:32             ` (EXT) " Matthias Schiffer
2020-08-26 10:34               ` Matthias Schiffer
2020-08-26 10:59               ` Fabio Estevam
2020-08-26 11:54                 ` (EXT) " Matthias Schiffer
2020-08-26 13:01                   ` Fabio Estevam
2020-08-26 13:13                     ` (EXT) " Matthias Schiffer
2020-08-26 13:49                       ` Fabio Estevam
2020-08-27  7:31                         ` Matthias Schiffer
2020-08-27 21:27                           ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).