Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add support for QCM6490 and QCS6490
From: Mohammad Rafi Shaik @ 2024-04-04  8:46 UTC (permalink / raw)
  To: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai
  Cc: linux-arm-msm, alsa-devel, linux-sound, devicetree, linux-kernel,
	quic_rohkumar, Mohammad Rafi Shaik

This patchset adds support for sound card on Qualcomm QCM6490 IDP and
QCS6490 RB3Gen2 boards.

Changes since v1:
	- Use existing sc8280xp machine driver instead of separate driver.
	- Modify qcs6490 compatible name as qcs6490-rb3gen2.

Mohammad Rafi Shaik (2):
  ASoC: dt-bindings: qcom,sm8250: Add QCM6490 snd QCS6490 sound card
  ASoC: qcom: sc8280xp: Add support for QCM6490 and QCS6490

 Documentation/devicetree/bindings/sound/qcom,sm8250.yaml | 2 ++
 sound/soc/qcom/sc8280xp.c                                | 2 ++
 2 files changed, 4 insertions(+)

-- 
2.25.1


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: arm: rockchip: Add Protonic MECSBC board
From: Krzysztof Kozlowski @ 2024-04-04  8:47 UTC (permalink / raw)
  To: Sascha Hauer, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Heiko Stuebner
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	David Jander
In-Reply-To: <20240404-protonic-mecsbc-v1-1-ad5b42ade6c6@pengutronix.de>

On 04/04/2024 10:34, Sascha Hauer wrote:
> MECSBC is a single board computer for blood analysis machines from
> RR-Mechatronics, designed and manufactured by Protonic Holland, based on
> the Rockchip RK3568 SoC.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH v2 1/2] ASoC: dt-bindings: qcom,sm8250: Add QCM6490 snd QCS6490 sound card
From: Mohammad Rafi Shaik @ 2024-04-04  8:46 UTC (permalink / raw)
  To: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai
  Cc: linux-arm-msm, alsa-devel, linux-sound, devicetree, linux-kernel,
	quic_rohkumar, Mohammad Rafi Shaik
In-Reply-To: <20240404084631.417779-1-quic_mohs@quicinc.com>

Document the bindings for the Qualcomm QCM6490 IDP and QCS6490 RB3Gen2
soc platforms sound card.

The bindings are the same as for other newer Qualcomm ADSP sound cards,
thus keep them in existing qcom,sm8250.yaml file, even though Linux driver
is separate.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 Documentation/devicetree/bindings/sound/qcom,sm8250.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
index 2ab6871e89e5..ff1a27f26bc2 100644
--- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
@@ -29,6 +29,8 @@ properties:
       - enum:
           - qcom,apq8016-sbc-sndcard
           - qcom,msm8916-qdsp6-sndcard
+          - qcom,qcm6490-sndcard
+          - qcom,qcs6490-rb3gen2-sndcard
           - qcom,qrb5165-rb5-sndcard
           - qcom,sc7180-qdsp6-sndcard
           - qcom,sc8280xp-sndcard
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH 2/4] arm64: dts: airoha: Add EN7581 clock node
From: AngeloGioacchino Del Regno @ 2024-04-04  8:47 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-clk
  Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83
In-Reply-To: <8465b7562bcf53a0adfdd4ae01b3ed94d6d5bc54.1712160869.git.lorenzo@kernel.org>

Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
> Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
> 
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
> index 55eb1762fb11..a1daaaef0de0 100644
> --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
> +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
> @@ -2,6 +2,7 @@
>   
>   #include <dt-bindings/interrupt-controller/irq.h>
>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/en7523-clk.h>
>   
>   / {
>   	interrupt-parent = <&gic>;
> @@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
>   			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
>   			clock-frequency = <1843200>;
>   		};
> +
> +		scu: system-controller@1fa20000 {

Uhm, why is this not a clock-controller but a system-controller?

Cheers,
Angelo

> +			compatible = "airoha,en7581-scu";
> +			reg = <0x0 0x1fa20000 0x0 0x400>,
> +			      <0x0 0x1fb00000 0x0 0x1000>,
> +			      <0x0 0x1fbe3400 0x0 0xfc>;
> +			#clock-cells = <1>;
> +		};
>   	};
>   };




^ permalink raw reply

* [PATCH v2 2/2] ASoC: qcom: sc8280xp: Add support for QCM6490 and QCS6490
From: Mohammad Rafi Shaik @ 2024-04-04  8:46 UTC (permalink / raw)
  To: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai
  Cc: linux-arm-msm, alsa-devel, linux-sound, devicetree, linux-kernel,
	quic_rohkumar, Mohammad Rafi Shaik
In-Reply-To: <20240404084631.417779-1-quic_mohs@quicinc.com>

Add compatibles for sound card on Qualcomm QCM6490 IDP and
QCS6490 RB3Gen2 boards.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 sound/soc/qcom/sc8280xp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index b7fd503a1666..09c949e01479 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -169,6 +169,8 @@ static int sc8280xp_platform_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id snd_sc8280xp_dt_match[] = {
+	{.compatible = "qcom,qcm6490-sndcard", "qcm6490"},
+	{.compatible = "qcom,qcs6490-rb3gen2-sndcard", "qcs6490"},
 	{.compatible = "qcom,sc8280xp-sndcard", "sc8280xp"},
 	{.compatible = "qcom,sm8450-sndcard", "sm8450"},
 	{.compatible = "qcom,sm8550-sndcard", "sm8550"},
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree
From: Krzysztof Kozlowski @ 2024-04-04  8:49 UTC (permalink / raw)
  To: Sascha Hauer, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Heiko Stuebner
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	David Jander
In-Reply-To: <20240404-protonic-mecsbc-v1-2-ad5b42ade6c6@pengutronix.de>

On 04/04/2024 10:34, Sascha Hauer wrote:
> From: David Jander <david@protonic.nl>
> 
> MECSBC is a single board computer for blood analysis machines from
> RR-Mechatronics, designed and manufactured by Protonic Holland, based on
> the Rockchip RK3568 SoC.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

...

> +	vdd_gpu: regulator-vdd-gpu {
> +		compatible = "pwm-regulator";
> +		pwms = <&pwm1 0 5000 PWM_POLARITY_INVERTED>;
> +		regulator-name = "vdd_gpu";
> +		regulator-min-microvolt = <915000>;
> +		regulator-max-microvolt = <1000000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-settling-time-up-us = <250>;
> +		pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> +	};
> +
> +	vdd_npu: regulator-vdd-npu {
> +		compatible = "pwm-regulator";
> +		pwms = <&pwm2 0 5000 PWM_POLARITY_INVERTED>;
> +		regulator-name = "vdd_npu";
> +		regulator-min-microvolt = <915000>;
> +		regulator-max-microvolt = <1000000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-settling-time-up-us = <250>;
> +		pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> +	};
> +
> +	p3v3: p3v3-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "p3v3";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	p1v8: p1v8-regulator {

Please keep consistent naming - your other regulators are
"regulator-foo", not "foo-regulator". The "regulator-foo" is preferred
usually, because it groups devices nicely.

> +		compatible = "regulator-fixed";
> +		regulator-name = "p1v8";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};


...

> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c3m0_xfer>;
> +	status = "okay";
> +
> +	tas2562: tas2562@4c {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

e.g. audio-codec, speaker, amplifier

> +		compatible = "ti,tas2562";
> +		reg = <0x4c>;
> +		#sound-dai-cells = <0>;
> +		shutdown-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
> +		interrupt-parent = <&gpio1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_tas2562>;
> +		interrupts = <RK_PD1 IRQ_TYPE_LEVEL_LOW>;
> +		ti,imon-slot-no = <0>;
> +	};
> +};
> +
> +&i2c5 {
> +	status = "okay";
> +
> +	tmp1075n@48 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +		compatible = "ti,tmp1075";
> +		reg = <0x48>;
> +	};
> +
> +	pcf8563: rtc@51 {
> +		compatible = "nxp,pcf85363";
> +		reg = <0x51>;
> +		#clock-cells = <0>;
> +		clock-output-names = "rtcic_32kout";
> +	};
> +};
> +

...

> +&pcie3x2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie30x2m1_pins>;
> +	reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> +	vpcie3v3-supply = <&p3v3>;
> +	status = "okay";
> +};
> +
> +&pinctrl {
> +	ethernet {
> +		eth_phy1_rst: eth_phy1_rst {

No underscores in node names.



Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/2] ASoC: dt-bindings: qcom,sm8250: Add QCM6490 snd QCS6490 sound card
From: Dmitry Baryshkov @ 2024-04-04  8:53 UTC (permalink / raw)
  To: Mohammad Rafi Shaik
  Cc: Srinivas Kandagatla, Banajit Goswami, Liam Girdwood, Mark Brown,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela,
	Takashi Iwai, linux-arm-msm, alsa-devel, linux-sound, devicetree,
	linux-kernel, quic_rohkumar
In-Reply-To: <20240404084631.417779-2-quic_mohs@quicinc.com>

On Thu, 4 Apr 2024 at 11:48, Mohammad Rafi Shaik <quic_mohs@quicinc.com> wrote:
>
> Document the bindings for the Qualcomm QCM6490 IDP and QCS6490 RB3Gen2
> soc platforms sound card.
>
> The bindings are the same as for other newer Qualcomm ADSP sound cards,
> thus keep them in existing qcom,sm8250.yaml file, even though Linux driver
> is separate.
>
> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
> ---
>  Documentation/devicetree/bindings/sound/qcom,sm8250.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> index 2ab6871e89e5..ff1a27f26bc2 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
> @@ -29,6 +29,8 @@ properties:
>        - enum:
>            - qcom,apq8016-sbc-sndcard
>            - qcom,msm8916-qdsp6-sndcard
> +          - qcom,qcm6490-sndcard
> +          - qcom,qcs6490-rb3gen2-sndcard

My 2c: you are adding one soundcard for the SoC family (qcm6490) and
another one for the particular board kind (qcs6490-rb3gen2). That
doesn't seem logical.

>            - qcom,qrb5165-rb5-sndcard
>            - qcom,sc7180-qdsp6-sndcard
>            - qcom,sc8280xp-sndcard
> --
> 2.25.1
>
>


-- 
With best wishes
Dmitry

^ permalink raw reply

* [PATCH v5 0/2] iio: temperature: ltc2983: small improvements
From: Nuno Sa @ 2024-04-04  8:58 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Krzysztof Kozlowski

Hi Jonathan,

As suggested by you, v5 only has the regulator stuff. I'll send another
series for the new dev_errp_helper().

---
Changes in v5:
- Dropped patches 1,2,5 and 6.
- Link to v4: https://lore.kernel.org/all/20240328-ltc2983-misc-improv-v4-0-0cc428c07cd5@analog.com/

---
Nuno Sa (2):
      dt-bindings: iio: temperature: ltc2983: document power supply
      iio: temperature: ltc2983: support vdd regulator

 Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml | 4 ++++
 drivers/iio/temperature/ltc2983.c                                  | 5 +++++
 2 files changed, 9 insertions(+)
---
base-commit: 6020ca4de8e5404b20f15a6d9873cd6eb5f6d8d6
change-id: 20240222-ltc2983-misc-improv-1c7a78ece93f
--

Thanks!
- Nuno Sá


^ permalink raw reply

* [PATCH v5 1/2] dt-bindings: iio: temperature: ltc2983: document power supply
From: Nuno Sa @ 2024-04-04  8:58 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Krzysztof Kozlowski
In-Reply-To: <20240404-ltc2983-misc-improv-v5-0-c1f58057fcea@analog.com>

Add a property for the VDD power supply regulator.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
index dbb85135fd66..312febeeb3bb 100644
--- a/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml
@@ -57,6 +57,8 @@ properties:
   interrupts:
     maxItems: 1
 
+  vdd-supply: true
+
   adi,mux-delay-config-us:
     description: |
       Extra delay prior to each conversion, in addition to the internal 1ms
@@ -460,6 +462,7 @@ required:
   - compatible
   - reg
   - interrupts
+  - vdd-supply
 
 additionalProperties: false
 
@@ -489,6 +492,7 @@ examples:
             #address-cells = <1>;
             #size-cells = <0>;
 
+            vdd-supply = <&supply>;
             interrupts = <20 IRQ_TYPE_EDGE_RISING>;
             interrupt-parent = <&gpio>;
 

-- 
2.44.0


^ permalink raw reply related

* [PATCH v5 2/2] iio: temperature: ltc2983: support vdd regulator
From: Nuno Sa @ 2024-04-04  8:58 UTC (permalink / raw)
  To: linux-iio, devicetree
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <20240404-ltc2983-misc-improv-v5-0-c1f58057fcea@analog.com>

Add support for the power supply regulator.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/temperature/ltc2983.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index 3c4524d57b8e..24d19f3c7292 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
 #include <asm/byteorder.h>
@@ -1597,6 +1598,10 @@ static int ltc2983_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = devm_regulator_get_enable(&spi->dev, "vdd");
+	if (ret)
+		return ret;
+
 	gpio = devm_gpiod_get_optional(&st->spi->dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(gpio))
 		return PTR_ERR(gpio);

-- 
2.44.0


^ permalink raw reply related

* Re: [PATCH v7 1/5] dt-bindings: interconnect: Add Qualcomm IPQ9574 support
From: Varadarajan Narayanan @ 2024-04-04  8:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: andersson, konrad.dybcio, mturquette, sboyd, robh, krzk+dt,
	conor+dt, djakov, dmitry.baryshkov, quic_anusha, linux-arm-msm,
	linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <58c9b754-b9a7-444d-9545-9e6648010630@kernel.org>

On Wed, Apr 03, 2024 at 04:59:40PM +0200, Krzysztof Kozlowski wrote:
> On 03/04/2024 12:42, Varadarajan Narayanan wrote:
> > Add interconnect-cells to clock provider so that it can be
> > used as icc provider.
> >
> > Add master/slave ids for Qualcomm IPQ9574 Network-On-Chip
> > interfaces. This will be used by the gcc-ipq9574 driver
> > that will for providing interconnect services using the
> > icc-clk framework.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v7:
> > Fix macro names to be consistent with other bindings
> > v6:
> > Removed Reviewed-by: Krzysztof Kozlowski
> > Redefine the bindings such that driver and DT can share them
> >
> > v3:
> > Squash Documentation/ and include/ changes into same patch
> >
> > qcom,ipq9574.h
> > 	Move 'first id' to clock driver
> >
> > ---
> >  .../bindings/clock/qcom,ipq9574-gcc.yaml      |  3 +
> >  .../dt-bindings/interconnect/qcom,ipq9574.h   | 87 +++++++++++++++++++
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > index 944a0ea79cd6..824781cbdf34 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > @@ -33,6 +33,9 @@ properties:
> >        - description: PCIE30 PHY3 pipe clock source
> >        - description: USB3 PHY pipe clock source
> >
> > +  '#interconnect-cells':
> > +    const: 1
> > +
> >  required:
> >    - compatible
> >    - clocks
> > diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > new file mode 100644
> > index 000000000000..0b076b0cf880
> > --- /dev/null
> > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > @@ -0,0 +1,87 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +#ifndef INTERCONNECT_QCOM_IPQ9574_H
> > +#define INTERCONNECT_QCOM_IPQ9574_H
> > +
> > +#define ICC_ANOC_PCIE0		0
> > +#define ICC_SNOC_PCIE0		1
> > +#define ICC_ANOC_PCIE1		2
> > +#define ICC_SNOC_PCIE1		3
> > +#define ICC_ANOC_PCIE2		4
> > +#define ICC_SNOC_PCIE2		5
> > +#define ICC_ANOC_PCIE3		6
> > +#define ICC_SNOC_PCIE3		7
> > +#define ICC_SNOC_USB		8
> > +#define ICC_ANOC_USB_AXI	9
> > +#define ICC_NSSNOC_NSSCC	10
> > +#define ICC_NSSNOC_SNOC_0	11
> > +#define ICC_NSSNOC_SNOC_1	12
> > +#define ICC_NSSNOC_PCNOC_1	13
> > +#define ICC_NSSNOC_QOSGEN_REF	14
> > +#define ICC_NSSNOC_TIMEOUT_REF	15
> > +#define ICC_NSSNOC_XO_DCD	16
> > +#define ICC_NSSNOC_ATB		17
> > +#define ICC_MEM_NOC_NSSNOC	18
> > +#define ICC_NSSNOC_MEMNOC	19
> > +#define ICC_NSSNOC_MEM_NOC_1	20
> > +
> > +#define ICC_NSSNOC_PPE		0
> > +#define ICC_NSSNOC_PPE_CFG	1
> > +#define ICC_NSSNOC_NSS_CSR	2
> > +#define ICC_NSSNOC_IMEM_QSB	3
> > +#define ICC_NSSNOC_IMEM_AHB	4
> > +
> > +#define MASTER_ANOC_PCIE0		(ICC_ANOC_PCIE0 * 2)
> > +#define SLAVE_ANOC_PCIE0		((ICC_ANOC_PCIE0 * 2) + 1)
>
> Which existing Qualcomm platform has such code?

Existing Qualcomm platforms don't use icc-clk. They use icc-rpm
or icc-rpmh. clk-cbf-msm8996.c is the only driver that uses icc-clk.

The icc_clk_register automatically creates master & slave nodes
for each clk entry provided as input with the node-ids 'n' and
'n+1'. Since clk-cbf-msm8996.c has only one entry, it could just
define MASTER_CBF_M4M and SLAVE_CBF_M4M with 0 and 1 and avoid these
calculations.

However, ipq9574 gives an array of clock entries as input to
icc_clk_register. To tie the order/sequence of these clock
entries correctly with the node-ids, this calculation is needed.

> This is the third time I am asking for consistent headers. Open
> existing, recently added headers and look how it is done there. Why?
> Because I am against such calculations and see no reason for them.

Apologies. Regret that I have to trouble you.

In this ipq9574 case, have to reconcile between the following
feedbacks.

1. https://lore.kernel.org/linux-arm-msm/fe40b307-26d0-4b2a-869b-5d093415b9d1@linaro.org/
   We could probably use indexed identifiers here to avoid confusion:
   [ICC_BINDING_NAME] = CLK_BINDING_NAME

2. https://lore.kernel.org/linux-arm-msm/95f4e99a60cc97770fc3cee850b62faf.sboyd@kernel.org/
   Are these supposed to be in a dt-binding header?

3. https://lore.kernel.org/linux-arm-msm/031d0a35-b192-4161-beef-97b89d5d1da6@linaro.org/
   Do you use them as well in the DTS?

Having the defines (with the calculations) seemed to to comply
with the above three feedbacks.

Please let me know if this can be handled in a different way that
would be consistent with other Qualcomm platforms.

Thanks
Varada

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree
From: Heiko Stübner @ 2024-04-04  8:56 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Sascha Hauer
  Cc: devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	David Jander, Sascha Hauer
In-Reply-To: <20240404-protonic-mecsbc-v1-2-ad5b42ade6c6@pengutronix.de>

Hi Sascha,

Am Donnerstag, 4. April 2024, 10:34:40 CEST schrieb Sascha Hauer:
> From: David Jander <david@protonic.nl>
> 
> MECSBC is a single board computer for blood analysis machines from
> RR-Mechatronics, designed and manufactured by Protonic Holland, based on
> the Rockchip RK3568 SoC.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm64/boot/dts/rockchip/Makefile          |   1 +
>  arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts | 394 +++++++++++++++++++++++++
>  2 files changed, 395 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index f906a868b71ac..1152e0f6a25cb 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -104,6 +104,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5c.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-nanopi-r5s.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-odroid-m1.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-qnap-ts433.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-mecsbc.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-radxa-e25.dtb

alphabetical sorting of entries please

>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-roc-pc.dtb
>  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-rock-3a.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
> new file mode 100644
> index 0000000000000..e50d135042ec7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-mecsbc.dts
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pinctrl/rockchip.h>
> +#include <dt-bindings/pwm/pwm.h>
> +#include "rk3568.dtsi"
> +
> +/ {
> +	model = "Protonic MECSBC";
> +	compatible = "prt,mecsbc", "rockchip,rk3568";
> +
> +	aliases {
> +		mmc0 = &sdhci;
> +		mmc1 = &sdmmc0;
> +	};
> +
> +	chosen: chosen {
> +		stdout-path = "serial2:1500000n8";
> +	};
> +
> +	tas2562-sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,name = "Speaker";
> +		simple-audio-card,mclk-fs = <256>;
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&i2s1_8ch>;
> +		};
> +
> +		simple-audio-card,codec {
> +			sound-dai = <&tas2562>;
> +		};
> +	};
> +
> +	vdd_gpu: regulator-vdd-gpu {
> +		compatible = "pwm-regulator";
> +		pwms = <&pwm1 0 5000 PWM_POLARITY_INVERTED>;
> +		regulator-name = "vdd_gpu";
> +		regulator-min-microvolt = <915000>;
> +		regulator-max-microvolt = <1000000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-settling-time-up-us = <250>;
> +		pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> +	};
> +
> +	vdd_npu: regulator-vdd-npu {
> +		compatible = "pwm-regulator";
> +		pwms = <&pwm2 0 5000 PWM_POLARITY_INVERTED>;
> +		regulator-name = "vdd_npu";
> +		regulator-min-microvolt = <915000>;
> +		regulator-max-microvolt = <1000000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-settling-time-up-us = <250>;
> +		pwm-dutycycle-range = <0 100>; /* dutycycle inverted 0% => 0.915V */
> +	};
> +
> +	p3v3: p3v3-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "p3v3";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	p1v8: p1v8-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "p1v8";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};

please sort alphabetical by node-name

> +};
> +
> +&combphy0 {
> +	status = "okay";
> +};
> +
> +&combphy1 {
> +	status = "okay";
> +};
> +
> +&combphy2 {
> +	status = "okay";
> +};
> +
> +&cpu0 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu1 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu2 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&cpu3 {
> +	cpu-supply = <&vdd_cpu>;
> +};
> +
> +&gmac1 {
> +	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> +	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> +	phy-handle = <&rgmii_phy1>;
> +	phy-mode = "rgmii";
> +	clock_in_out = "output";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac1m1_miim
> +		     &gmac1m1_tx_bus2
> +		     &gmac1m1_rx_bus2
> +		     &gmac1m1_rgmii_clk
> +		     &gmac1m1_clkinout
> +		     &gmac1m1_rgmii_bus>;
> +	status = "okay";
> +	tx_delay = <0x30>;
> +	rx_delay = <0x10>;
> +};
> +
> +&gpu {
> +	mali-supply = <&vdd_gpu>;
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	status = "okay";
> +
> +	vdd_cpu: regulator@60 {
> +		compatible = "fcs,fan53555";
> +		reg = <0x60>;
> +		fcs,suspend-voltage-selector = <1>;
> +		regulator-name = "vdd_cpu";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <800000>;
> +		regulator-max-microvolt = <1150000>;
> +		regulator-ramp-delay = <2300>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +};
> +
> +&i2c2 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c2m0_xfer>;
> +};
> +
> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c3m0_xfer>;
> +	status = "okay";
> +
> +	tas2562: tas2562@4c {
> +		compatible = "ti,tas2562";
> +		reg = <0x4c>;
> +		#sound-dai-cells = <0>;
> +		shutdown-gpios = <&gpio1 RK_PD4 GPIO_ACTIVE_HIGH>;
> +		interrupt-parent = <&gpio1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_tas2562>;
> +		interrupts = <RK_PD1 IRQ_TYPE_LEVEL_LOW>;
> +		ti,imon-slot-no = <0>;
> +	};
> +};
> +
> +&i2c5 {
> +	status = "okay";
> +
> +	tmp1075n@48 {
> +		compatible = "ti,tmp1075";
> +		reg = <0x48>;
> +	};
> +
> +	pcf8563: rtc@51 {
> +		compatible = "nxp,pcf85363";
> +		reg = <0x51>;
> +		#clock-cells = <0>;
> +		clock-output-names = "rtcic_32kout";
> +	};
> +};
> +
> +&i2s1_8ch {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2s1m0_sclktx &i2s1m0_lrcktx &i2s1m0_sdi0 &i2s1m0_sdo0>;
> +	rockchip,trcm-sync-tx-only;
> +	status = "okay";
> +};
> +
> +&mdio1 {
> +	rgmii_phy1: ethernet-phy@2 {
> +		compatible = "ethernet-phy-ieee802.3-c22";
> +		reg = <0x2>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&eth_phy1_rst>;
> +		reset-assert-us = <20000>;
> +		reset-deassert-us = <100000>;
> +		reset-gpios = <&gpio4 RK_PB3 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +&pcie2x1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie20m1_pins>;
> +	reset-gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_HIGH>;
> +	status = "okay";
> +};
> +
> +&pcie30phy {
> +	status = "okay";
> +};
> +
> +&pcie3x2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie30x2m1_pins>;
> +	reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> +	vpcie3v3-supply = <&p3v3>;
> +	status = "okay";
> +};
> +
> +&pinctrl {
> +	ethernet {
> +		eth_phy1_rst: eth_phy1_rst {
> +			rockchip,pins = <4 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
> +	tas2562 {
> +		pinctrl_tas2562: tas2562 {
> +			rockchip,pins = <1 RK_PD4 RK_FUNC_GPIO &pcfg_pull_up>;
> +		};
> +	};
> +};
> +
> +&pmu_io_domains {
> +	pmuio1-supply = <&p3v3>;
> +	pmuio2-supply = <&p3v3>;
> +	vccio1-supply = <&p1v8>;
> +	vccio2-supply = <&p1v8>;
> +	vccio3-supply = <&p3v3>;
> +	vccio4-supply = <&p1v8>;
> +	vccio5-supply = <&p3v3>;
> +	vccio6-supply = <&p1v8>;
> +	vccio7-supply = <&p3v3>;
> +	status = "okay";
> +};
> +
> +&saradc {
> +	vref-supply = <&p1v8>;
> +	status = "okay";
> +};
> +
> +&sdhci {
> +	bus-width = <8>;
> +	max-frequency = <200000000>;
> +	non-removable;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_datastrobe>;
> +	vmmc-supply = <&p3v3>;
> +	vqmmc-supply = <&p1v8>;
> +	mmc-hs200-1_8v;
> +	non-removable;
> +	no-sd;
> +	no-sdio;
> +	status = "okay";
> +};
> +
> +&sdmmc0 {
> +	bus-width = <4>;
> +	cap-sd-highspeed;
> +	cd-gpios = <&gpio0 RK_PA4 GPIO_ACTIVE_LOW>;
> +	disable-wp;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sdmmc0_bus4 &sdmmc0_clk &sdmmc0_cmd &sdmmc0_det>;
> +	sd-uhs-sdr50;
> +	vmmc-supply = <&p3v3>;
> +	vqmmc-supply = <&p3v3>;
> +	status = "okay";
> +};
> +
> +&tsadc {
> +	rockchip,hw-tshut-mode = <1>;
> +	rockchip,hw-tshut-polarity = <0>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	status = "okay";
> +};
> +
> +&usb_host0_ehci {
> +	status = "okay";
> +};
> +
> +&usb_host0_ohci {
> +	status = "okay";
> +};
> +
> +&usb_host0_xhci {
> +	extcon = <&usb2phy0>;
> +	status = "okay";
> +	dr_mode = "host";

please sort properties alphabetical, with
compatible at the top and status last.


> +};
> +
> +&usb_host1_ehci {
> +	status = "okay";
> +};
> +
> +&usb_host1_ohci {
> +	status = "okay";
> +};
> +
> +&usb_host1_xhci {
> +	status = "okay";
> +};
> +
> +&usb2phy0 {
> +	status = "okay";
> +};
> +
> +&usb2phy0_host {
> +	status = "okay";
> +};
> +
> +&usb2phy0_otg {
> +	status = "okay";
> +};
> +
> +&usb2phy1 {
> +	status = "okay";
> +};
> +
> +&usb2phy1_host {
> +	status = "okay";
> +};
> +
> +&usb2phy1_otg {
> +	status = "okay";
> +};
> +
> +&pwm1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm1m0_pins>;
> +};
> +
> +&pwm2 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pwm2m0_pins>;
> +};

please sort phandles "&pwm2" alphabetical and status comes last


> +
> +&gpu_opp_table {
> +	compatible = "operating-points-v2";
> +
> +	opp-200000000 {
> +		opp-hz = /bits/ 64 <200000000>;
> +		opp-microvolt = <915000>;
> +	};
> +
> +	opp-300000000 {
> +		opp-hz = /bits/ 64 <300000000>;
> +		opp-microvolt = <915000>;
> +	};
> +
> +	opp-400000000 {
> +		opp-hz = /bits/ 64 <400000000>;
> +		opp-microvolt = <915000>;
> +	};
> +
> +	opp-600000000 {
> +		opp-hz = /bits/ 64 <600000000>;
> +		opp-microvolt = <920000>;
> +	};
> +
> +	opp-700000000 {
> +		opp-hz = /bits/ 64 <700000000>;
> +		opp-microvolt = <950000>;
> +	};
> +
> +	opp-800000000 {
> +		opp-hz = /bits/ 64 <800000000>;
> +		opp-microvolt = <1000000>;
> +	};
> +};

a comment would be nice, why the OPPs get changed


Heiko



^ permalink raw reply

* Re: [PATCH 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-04  8:57 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-clk, mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83
In-Reply-To: <abff4844-b444-48cc-8dad-18eefa6c386c@collabora.com>

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

> Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
> > Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
> > 
> > Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >   arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
> > index 55eb1762fb11..a1daaaef0de0 100644
> > --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
> > +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
> > @@ -2,6 +2,7 @@
> >   #include <dt-bindings/interrupt-controller/irq.h>
> >   #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <dt-bindings/clock/en7523-clk.h>
> >   / {
> >   	interrupt-parent = <&gic>;
> > @@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
> >   			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> >   			clock-frequency = <1843200>;
> >   		};
> > +
> > +		scu: system-controller@1fa20000 {
> 
> Uhm, why is this not a clock-controller but a system-controller?

I used the same approach used for en7523.dtsi. I guess it is done
that way because the registers come from scu (system control unit)
regmap, but I guess we can use clock-controller instead.

Regards,
Lorenzo

> 
> Cheers,
> Angelo
> 
> > +			compatible = "airoha,en7581-scu";
> > +			reg = <0x0 0x1fa20000 0x0 0x400>,
> > +			      <0x0 0x1fb00000 0x0 0x1000>,
> > +			      <0x0 0x1fbe3400 0x0 0xfc>;
> > +			#clock-cells = <1>;
> > +		};
> >   	};
> >   };
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH 4/4] clk: en7523: add EN7581 support
From: AngeloGioacchino Del Regno @ 2024-04-04  9:09 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-clk
  Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83
In-Reply-To: <3aaf638b846ecfdbfc1c903206b7d519d56c9130.1712160869.git.lorenzo@kernel.org>

Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
> Introduce EN7581 clock support to clk-en7523 driver.
> 
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   drivers/clk/clk-en7523.c | 130 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 125 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index c7def87b74c6..51a6c0cc7f58 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c
> @@ -4,13 +4,16 @@
>   #include <linux/clk-provider.h>
>   #include <linux/io.h>
>   #include <linux/of.h>
> +#include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <dt-bindings/clock/en7523-clk.h>
>   
>   #define REG_PCI_CONTROL			0x88
>   #define   REG_PCI_CONTROL_PERSTOUT	BIT(29)
>   #define   REG_PCI_CONTROL_PERSTOUT1	BIT(26)
> +#define   REG_PCI_CONTROL_REFCLK_EN0	BIT(23)
>   #define   REG_PCI_CONTROL_REFCLK_EN1	BIT(22)
> +#define   REG_PCI_CONTROL_PERSTOUT2	BIT(16)
>   #define REG_GSW_CLK_DIV_SEL		0x1b4
>   #define REG_EMI_CLK_DIV_SEL		0x1b8
>   #define REG_BUS_CLK_DIV_SEL		0x1bc
> @@ -18,10 +21,25 @@
>   #define REG_SPI_CLK_FREQ_SEL		0x1c8
>   #define REG_NPU_CLK_DIV_SEL		0x1fc
>   #define REG_CRYPTO_CLKSRC		0x200
> -#define REG_RESET_CONTROL		0x834
> +#define REG_RESET_CONTROL2		0x830

Wait what? The RESET2 register comes before RESET1 ?!?!

Is this a typo? :-)

> +#define   REG_RESET2_CONTROL_PCIE2	BIT(27)
> +#define REG_RESET_CONTROL1		0x834
>   #define   REG_RESET_CONTROL_PCIEHB	BIT(29)
>   #define   REG_RESET_CONTROL_PCIE1	BIT(27)
>   #define   REG_RESET_CONTROL_PCIE2	BIT(26)
> +/* EN7581 */
> +#define REG_PCIE0_MEM			0x00
> +#define REG_PCIE0_MEM_MASK		0x04
> +#define REG_PCIE1_MEM			0x08
> +#define REG_PCIE1_MEM_MASK		0x0c
> +#define REG_PCIE2_MEM			0x10
> +#define REG_PCIE2_MEM_MASK		0x14
> +#define REG_PCIE_RESET_OPEN_DRAIN	0x018c
> +#define REG_PCIE_RESET_OPEN_DRAIN_MASK	GENMASK(2, 0)
> +#define REG_NP_SCU_PCIC			0x88
> +#define REG_NP_SCU_SSTR			0x9c
> +#define REG_PCIE_XSI0_SEL_MASK		GENMASK(14, 13)
> +#define REG_PCIE_XSI1_SEL_MASK		GENMASK(12, 11)
>   
>   struct en_clk_desc {
>   	int id;
> @@ -207,14 +225,14 @@ static int en7523_pci_prepare(struct clk_hw *hw)
>   	usleep_range(1000, 2000);
>   
>   	/* Reset to default */
> -	val = readl(np_base + REG_RESET_CONTROL);
> +	val = readl(np_base + REG_RESET_CONTROL1);
>   	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
>   	       REG_RESET_CONTROL_PCIEHB;
> -	writel(val & ~mask, np_base + REG_RESET_CONTROL);
> +	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
>   	usleep_range(1000, 2000);
> -	writel(val | mask, np_base + REG_RESET_CONTROL);
> +	writel(val | mask, np_base + REG_RESET_CONTROL1);
>   	msleep(100);
> -	writel(val & ~mask, np_base + REG_RESET_CONTROL);
> +	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
>   	usleep_range(5000, 10000);
>   
>   	/* Release device */
> @@ -262,6 +280,64 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
>   	return &cg->hw;
>   }
>   
> +static int en7581_pci_is_enabled(struct clk_hw *hw)
> +{
> +	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> +	u32 val, mask;
> +
> +	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
> +	val = readl(cg->base + REG_PCI_CONTROL);
> +	return (val & mask) == mask;
> +}
> +
> +static int en7581_pci_prepare(struct clk_hw *hw)
> +{
> +	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> +	void __iomem *np_base = cg->base;
> +	u32 val, mask;
> +
> +	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
> +	       REG_RESET_CONTROL_PCIEHB;
> +	val = readl(np_base + REG_RESET_CONTROL1);
> +	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
> +	val = readl(np_base + REG_RESET_CONTROL2);
> +	writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
> +	usleep_range(5000, 10000);
> +
> +	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
> +	       REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
> +	       REG_PCI_CONTROL_PERSTOUT;

I'm not sure that this is actually something to control in a clock driver...

the right thing to do would be to add a reset controller to this clock driver
and then assert/deassert reset in the PCIe PHY/MAC driver.

Perhaps REFCLK_EN0/EN1 can be manipulated in a .enable() callback, treating
this really just as what it appears to really be: a gate clock! (hint: check
clk-gate.c)

> +	val = readl(np_base + REG_PCI_CONTROL);
> +	writel(val | mask, np_base + REG_PCI_CONTROL);
> +	msleep(250);
> +
> +	return 0;
> +}
> +
> +static void en7581_pci_unprepare(struct clk_hw *hw)
> +{
> +	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> +	void __iomem *np_base = cg->base;
> +	u32 val, mask;
> +
> +	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |

...and this should be a clk-gate .disable() callback, I guess :-)

> +	       REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
> +	       REG_PCI_CONTROL_PERSTOUT;
> +	val = readl(np_base + REG_PCI_CONTROL);
> +	writel(val & ~mask, np_base + REG_PCI_CONTROL);
> +	usleep_range(1000, 2000);
> +
> +	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
> +	       REG_RESET_CONTROL_PCIEHB;
> +	val = readl(np_base + REG_RESET_CONTROL1);
> +	writel(val | mask, np_base + REG_RESET_CONTROL1);
> +	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
> +	writel(val | mask, np_base + REG_RESET_CONTROL1);
> +	val = readl(np_base + REG_RESET_CONTROL2);
> +	writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
> +	msleep(100);
> +}
> +
>   static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
>   				   void __iomem *base, void __iomem *np_base)
>   {
> @@ -291,6 +367,37 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
>   	clk_data->num = EN7523_NUM_CLOCKS;
>   }
>   
> +static int en7581_clk_hw_init(struct platform_device *pdev,
> +			      void __iomem *base,
> +			      void __iomem *np_base)
> +{
> +	void __iomem *pb_base;
> +	u32 val;
> +
> +	pb_base = devm_platform_ioremap_resource(pdev, 2);
> +	if (IS_ERR(pb_base))
> +		return PTR_ERR(pb_base);
> +
> +	val = readl(np_base + REG_NP_SCU_SSTR);
> +	val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
> +	writel(val, np_base + REG_NP_SCU_SSTR);
> +	val = readl(np_base + REG_NP_SCU_PCIC);
> +	writel(val | 3, np_base + REG_NP_SCU_PCIC);

What is 3?

#define SOMETHING 3 ??

> +
> +	writel(0x20000000, pb_base + REG_PCIE0_MEM);
> +	writel(0xfc000000, pb_base + REG_PCIE0_MEM_MASK);
> +	writel(0x24000000, pb_base + REG_PCIE1_MEM);
> +	writel(0xfc000000, pb_base + REG_PCIE1_MEM_MASK);
> +	writel(0x28000000, pb_base + REG_PCIE2_MEM);
> +	writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);

And... this is .. some BIT() and some GENMASK() as far as I understand...
do we have any clue about what you're setting to those registers?

Can MediaTek/Airoha help with this, please?

#define SOMETHING BIT(29) /* this is 0x20000000 */
#define SOME_MASK GENMASK(31, 26) /* this is 0xfc00000 */

> +
> +	val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
> +	writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
> +	       base + REG_PCIE_RESET_OPEN_DRAIN);
> +
> +	return 0;
> +}
> +
>   static int en7523_clk_probe(struct platform_device *pdev)
>   {
>   	struct device_node *node = pdev->dev.of_node;
> @@ -306,6 +413,12 @@ static int en7523_clk_probe(struct platform_device *pdev)
>   	if (IS_ERR(np_base))
>   		return PTR_ERR(np_base);
>   
> +	if (of_device_is_compatible(node, "airoha,en7581-scu")) {
> +		r = en7581_clk_hw_init(pdev, base, np_base);
> +		if (r)
> +			return r;
> +	}
> +
>   	clk_data = devm_kzalloc(&pdev->dev,
>   				struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
>   				GFP_KERNEL);
> @@ -329,8 +442,15 @@ static const struct clk_ops en7523_pcie_ops = {
>   	.unprepare = en7523_pci_unprepare,
>   };
>   

static const struct clk_en7523_pdata en7581_pdata = {
	.init = en7581_clk_hw_init, /* if (pdata->init) pdata->init(x, y, z) */
	.ops = en7581_pcie_ops,
};

or, alternatively:

static const struct .... = {
	.init = ...,
	.ops = (const struct clk_ops) {
		.is_enabled = en7581_pci_is_enabled,
		.... etc
	}
};

Cheers,
Angelo

> +static const struct clk_ops en7581_pcie_ops = {
> +	.is_enabled = en7581_pci_is_enabled,
> +	.prepare = en7581_pci_prepare,
> +	.unprepare = en7581_pci_unprepare,
> +};
> +
>   static const struct of_device_id of_match_clk_en7523[] = {
>   	{ .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
> +	{ .compatible = "airoha,en7581-scu", .data = &en7581_pcie_ops },
>   	{ /* sentinel */ }
>   };
>   

-

^ permalink raw reply

* Re: [PATCH 2/4] arm64: dts: airoha: Add EN7581 clock node
From: AngeloGioacchino Del Regno @ 2024-04-04  9:12 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-clk, mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83
In-Reply-To: <Zg5rc2GIwpN7f9Z2@lore-desk>

Il 04/04/24 10:57, Lorenzo Bianconi ha scritto:
>> Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
>>> Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
>>>
>>> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> ---
>>>    arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
>>> index 55eb1762fb11..a1daaaef0de0 100644
>>> --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
>>> +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
>>> @@ -2,6 +2,7 @@
>>>    #include <dt-bindings/interrupt-controller/irq.h>
>>>    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +#include <dt-bindings/clock/en7523-clk.h>
>>>    / {
>>>    	interrupt-parent = <&gic>;
>>> @@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
>>>    			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
>>>    			clock-frequency = <1843200>;
>>>    		};
>>> +
>>> +		scu: system-controller@1fa20000 {
>>
>> Uhm, why is this not a clock-controller but a system-controller?
> 
> I used the same approach used for en7523.dtsi. I guess it is done
> that way because the registers come from scu (system control unit)
> regmap, but I guess we can use clock-controller instead.
> 

Yeah, comes from there but you're actually defining a node for a clock-controller,
not a system-controller... makes sense to define this as

	scuclk: clock-controller@1fa20000

...or something along that line (for the phandle) so that, if another scu related
node appears for whatever reason, we distinguish between scuxyz and scuclk.

Cheers

> Regards,
> Lorenzo
> 
>>
>> Cheers,
>> Angelo
>>
>>> +			compatible = "airoha,en7581-scu";
>>> +			reg = <0x0 0x1fa20000 0x0 0x400>,
>>> +			      <0x0 0x1fb00000 0x0 0x1000>,
>>> +			      <0x0 0x1fbe3400 0x0 0xfc>;
>>> +			#clock-cells = <1>;
>>> +		};
>>>    	};
>>>    };
>>
>>
>>



^ permalink raw reply

* Re: [PATCH v2] arm64: dts: ti: k3-am62p: use eFuse MAC Address for CPSW3G Port 1
From: Siddharth Vadapalli @ 2024-04-04  9:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski, afd
  Cc: Siddharth Vadapalli, nm, vigneshr, kristo, robh, krzk+dt,
	conor+dt, devicetree, linux-kernel, linux-arm-kernel, srk
In-Reply-To: <18eb0e55-38ad-44f9-90b7-1917d8c0d5bb@linaro.org>

On Thu, Apr 04, 2024 at 10:43:04AM +0200, Krzysztof Kozlowski wrote:
> On 04/04/2024 10:18, Siddharth Vadapalli wrote:
> > Add the "cpsw-mac-efuse" node within "wkup_conf" node corresponding to the
> > CTRLMMR_MAC_IDx registers within the CTRL_MMR space. Assign the compatible
> > "ti,am62p-cpsw-mac-efuse" to enable "syscon_regmap" operations on these
> > registers. The MAC Address programmed in the eFuse is accessible through
> > the CTRLMMR_MAC_IDx registers. The "ti,syscon-efuse" device-tree property
> > points to the CTRLMMR_MAC_IDx registers, allowing the CPSW driver to fetch
> > the MAC Address and assign it to the network interface associated with
> > CPSW3G MAC Port 1.
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > ---
> > 
> > This patch is based on linux-next tagged next-20240404.
> > Patch depends on:
> > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@ti.com/
> > for the newly added "ti,am62p-cpsw-mac-efuse" compatible.
> > 
> > v1:
> > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402094200.4036076-1-s-vadapalli@ti.com/
> > Changes since v1:
> > - Since "wkup_conf" is modelled as a "simple-bus" rather than being
> 
> And maybe the hardware representation is not correct? What bus is it?

I will let Andrew comment on it. Andrew had posted a patch at:
https://lore.kernel.org/r/20240124184722.150615-10-afd@ti.com/
to convert an equivalent "main_conf" node for AM62 SoC to "simple-bus"
from the existing "syscon".

> 
> >   modelled as a System Controller node with the "syscon" compatible,
> >   directly passing the reference to the "wkup_conf" node using the
> >   "ti,syscon-efuse" device-tree property will not work.
> >   Therefore, I posted the patch at:
> >   https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@ti.com/
> >   in order to add a new compatible to be used for modelling the
> >   CTRLMMR_MAC_IDx registers as System Controller nodes, thereby
> >   allowing the existing "ti,syscon-efuse" property to be used.
> >   Now, "ti,syscon-efuse" points to the "cpsw_mac_efuse" node within
> >   "wkup_conf" node, with "cpsw_mac_efuse" being a "syscon" node.
> > 
> > Logs verifying that the CPSW driver assigns the MAC Address from the
> > eFuse based on the CTRLMMR_MAC_IDx registers at 0x43000200 and 0x43000204
> > to the interface eth0 corresponding to CPSW3G MAC Port 1:
> > https://gist.github.com/Siddharth-Vadapalli-at-TI/9982c6f13bf9b8cfaf97e8517e7dea13
> > 
> > Regards,
> > Siddharth.
> > 
> >  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi   | 1 +
> >  arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 5 +++++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> > index 7337a9e13535..848ca454a411 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> > @@ -696,6 +696,7 @@ cpsw_port1: port@1 {
> >  				label = "port1";
> >  				phys = <&phy_gmii_sel 1>;
> >  				mac-address = [00 00 00 00 00 00];
> > +				ti,syscon-efuse = <&cpsw_mac_efuse 0x0>;
> 
> Why this is not nvmem cell, like or efuses?

Since it belongs to the MMIO register set. You had recommended *not*
using nvmem for such MMIO registers at:
https://lore.kernel.org/r/48902771-5d3b-448a-8a74-ac18fb4f1a86@linaro.org/
"nvmem is for non-volatile memory, like OCOTP and eFUSE. This is not for
accessing regular MMIO registers of system-controller..."

Despite the "ti,syscon-efuse" property containing the term "efuse" in its
name, it is reading the CTRLMMR_MAC_IDx MMIO registers. So I assumed that
the existing approach which has been used on all K3 SoCs apart from this
one, will be suitable for this SoC as well.

> 
> >  			};
> >  
> >  			cpsw_port2: port@2 {
> > diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> > index a84756c336d0..df9d40f64e3b 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> > @@ -18,6 +18,11 @@ chipid: chipid@14 {
> >  			reg = <0x14 0x4>;
> >  			bootph-all;
> >  		};
> > +
> > +		cpsw_mac_efuse: cpsw-mac-efuse@200 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

I was following the convention that other mfd-syscon compatible nodes
seemed to be using:
https://github.com/torvalds/linux/blob/41bccc98fb7931d63d03f326a746ac4d429c1dd3/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#L502
The node is:
dss_oldi_io_ctrl: dss-oldi-io-ctrl@41e0
corresponding to the compatible:
"ti,am654-dss-oldi-io-ctrl"
which was added by commit:
https://github.com/torvalds/linux/commit/cb523495ee2a5938fbdd30b8a35094d386c55c12

Regards,
Siddharth.

^ permalink raw reply

* Re: [PATCH 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-04  9:20 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: linux-clk, mturquette, sboyd, linux-arm-kernel, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
	catalin.marinas, will, upstream, lorenzo.bianconi83
In-Reply-To: <d8794345-6014-4949-8e6b-e09bc0a1458f@collabora.com>

[-- Attachment #1: Type: text/plain, Size: 2307 bytes --]

> Il 04/04/24 10:57, Lorenzo Bianconi ha scritto:
> > > Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
> > > > Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
> > > > 
> > > > Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > >    arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
> > > >    1 file changed, 9 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
> > > > index 55eb1762fb11..a1daaaef0de0 100644
> > > > --- a/arch/arm64/boot/dts/airoha/en7581.dtsi
> > > > +++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
> > > > @@ -2,6 +2,7 @@
> > > >    #include <dt-bindings/interrupt-controller/irq.h>
> > > >    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +#include <dt-bindings/clock/en7523-clk.h>
> > > >    / {
> > > >    	interrupt-parent = <&gic>;
> > > > @@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
> > > >    			interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> > > >    			clock-frequency = <1843200>;
> > > >    		};
> > > > +
> > > > +		scu: system-controller@1fa20000 {
> > > 
> > > Uhm, why is this not a clock-controller but a system-controller?
> > 
> > I used the same approach used for en7523.dtsi. I guess it is done
> > that way because the registers come from scu (system control unit)
> > regmap, but I guess we can use clock-controller instead.
> > 
> 
> Yeah, comes from there but you're actually defining a node for a clock-controller,
> not a system-controller... makes sense to define this as
> 
> 	scuclk: clock-controller@1fa20000
> 
> ...or something along that line (for the phandle) so that, if another scu related
> node appears for whatever reason, we distinguish between scuxyz and scuclk.

ack, I agree. I will fix it.

Regards,
Lorenzo

> 
> Cheers
> 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > Cheers,
> > > Angelo
> > > 
> > > > +			compatible = "airoha,en7581-scu";
> > > > +			reg = <0x0 0x1fa20000 0x0 0x400>,
> > > > +			      <0x0 0x1fb00000 0x0 0x1000>,
> > > > +			      <0x0 0x1fbe3400 0x0 0xfc>;
> > > > +			#clock-cells = <1>;
> > > > +		};
> > > >    	};
> > > >    };
> > > 
> > > 
> > > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs
From: Kory Maincent @ 2024-04-04  9:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Jonathan Corbet, Luis Chamberlain, Russ Weight,
	Greg Kroah-Hartman, Rafael J. Wysocki, Krzysztof Kozlowski,
	Conor Dooley, Oleksij Rempel, Mark Brown, Frank Rowand,
	Andrew Lunn, Heiner Kallweit, Russell King, Thomas Petazzoni,
	netdev, linux-kernel, linux-doc, devicetree, Dent Project
In-Reply-To: <20240403143142.GA3508225-robh@kernel.org>

On Wed, 3 Apr 2024 09:31:42 -0500
Rob Herring <robh@kernel.org> wrote:

> >   
> > > > +
> > > > +          polarity-supported:
> > > > +            $ref: /schemas/types.yaml#/definitions/string-array
> > > > +            description:
> > > > +              Polarity configuration supported by the PSE PI pairsets.
> > > > +            minItems: 1
> > > > +            maxItems: 4
> > > > +            items:
> > > > +              enum:
> > > > +                - MDI-X
> > > > +                - MDI
> > > > +                - X
> > > > +                - S
> > > > +
> > > > +          vpwr-supply:
> > > > +            description: Regulator power supply for the PSE PI.    
> > > 
> > > I don't see this being used anywhere.  
> > 
> > Right, I forgot to add it to the PD692x0 and TPS23881 binding example!  
> 
> But is this really common/generic? I would think input power rails would 
> be chip specific.

I think as each PSE PI are seen as a regulator we may want it generic to track
each PI parent. Having the parent regulator described like that would force the
devicetree to describe where the power come from.
In contrary, for example, on the pd692x0 controller the regulators are connected
to the managers (PD69208) and not directly to the PIs. So the devicetree would
not really fit the hardware. It is indeed chip specific but having described
like that would be more simple.

If we decided to make it chip specific the core would have a callback to ask
the driver to fill the regulator_init_data structure for each PI before
registering the regulators. It is feasible.

Mmh in fact I am still unsure about the solution.

Oleksij as you were the first to push the idea. Have you more argument in mind
to make it generic?
https://lore.kernel.org/netdev/ZeObuKHkPN3tiWz_@pengutronix.de/

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v1 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path
From: Rob Herring @ 2024-04-04  9:26 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: shawn.sung, ck.hu, linux-arm-kernel, krzysztof.kozlowski+dt,
	chunkuang.hu, daniel, p.zabel, wenst, matthias.bgg, airlied,
	tzimmermann, kernel, maarten.lankhorst, dri-devel, linux-kernel,
	yu-chang.lee, jitao.shi, conor+dt, linux-mediatek, devicetree,
	mripard
In-Reply-To: <20240404081635.91412-3-angelogioacchino.delregno@collabora.com>


On Thu, 04 Apr 2024 10:16:34 +0200, AngeloGioacchino Del Regno wrote:
> Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
> per HW instance (so potentially up to six displays for multi-vdo SoCs).
> 
> The MMSYS or VDOSYS is always the first component in the DDP pipeline,
> so it only supports an output port with multiple endpoints - where each
> endpoint defines the starting point for one of the (currently three)
> possible hardware paths.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  .../bindings/arm/mediatek/mediatek,mmsys.yaml | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml: properties:port:properties:required: ['endpoint@0'] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml: properties:port:properties: 'required' should not be valid under {'$ref': '#/definitions/json-schema-prop-names'}
	hint: A json-schema keyword was found instead of a DT property name.
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml: properties:port:properties:required: ['endpoint@0'] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml: ignoring, error in schema: properties: port: properties: required
Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.example.dtb: /example-0/syscon@14000000: failed to match any schema with compatible: ['mediatek,mt8173-mmsys', 'syscon']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240404081635.91412-3-angelogioacchino.delregno@collabora.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [PATCH 06/10] dt-bindings: iio: dac: add bindings doc for AXI DAC driver
From: Nuno Sá @ 2024-04-04 10:03 UTC (permalink / raw)
  To: Rob Herring, Nuno Sa
  Cc: linux-iio, devicetree, Dragos Bogdan, Jonathan Cameron,
	Lars-Peter Clausen, Michael Hennerich, Krzysztof Kozlowski,
	Conor Dooley, Olivier Moysan
In-Reply-To: <20240401135948.GA349128-robh@kernel.org>

On Mon, 2024-04-01 at 08:59 -0500, Rob Herring wrote:
> On Thu, Mar 28, 2024 at 02:22:30PM +0100, Nuno Sa wrote:
> > This adds the bindings documentation for the AXI DAC driver.
> 
> Bindings are for h/w blocks, not 'drivers'.
> 
> Reword the subject to only say 'bindings' once.
> 
> 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  .../devicetree/bindings/iio/dac/adi,axi-dac.yaml   | 62
> > ++++++++++++++++++++++
> >  MAINTAINERS                                        |  7 +++
> >  2 files changed, 69 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > new file mode 100644
> > index 000000000000..1018fd274f04
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,axi-dac.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AXI DAC IP core
> > +
> > +maintainers:
> > +  - Nuno Sa <nuno.sa@analog.com>
> > +
> > +description: |
> > +  Analog Devices Generic AXI DAC IP core for interfacing a DAC device
> > +  with a high speed serial (JESD204B/C) or source synchronous parallel
> > +  interface (LVDS/CMOS).
> > +  Usually, some other interface type (i.e SPI) is used as a control
> > +  interface for the actual DAC, while this IP core will interface
> > +  to the data-lines of the DAC and handle the streaming of data into
> > +  memory via DMA.
> > +
> > +  https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,axi-dac-9.1.b
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  dmas:
> > +    maxItems: 1
> > +
> > +  dma-names:
> > +    items:
> > +      - const: tx
> 
> You don't need *-names if there is only 1 entry.

Looking at the dmaengine code [1], it actually looks like I should have this
property required, no?

[1]: https://elixir.bootlin.com/linux/v6.9-rc2/source/drivers/dma/of-dma.c#L270

- Nuno Sá



^ permalink raw reply

* Re: [PATCH v2] arm64: dts: ti: k3-am62p: use eFuse MAC Address for CPSW3G Port 1
From: Krzysztof Kozlowski @ 2024-04-04 10:00 UTC (permalink / raw)
  To: Siddharth Vadapalli, afd
  Cc: nm, vigneshr, kristo, robh, krzk+dt, conor+dt, devicetree,
	linux-kernel, linux-arm-kernel, srk
In-Reply-To: <75b53dda-23aa-4915-944a-4d9a619bd165@ti.com>

On 04/04/2024 11:12, Siddharth Vadapalli wrote:
> On Thu, Apr 04, 2024 at 10:43:04AM +0200, Krzysztof Kozlowski wrote:
>> On 04/04/2024 10:18, Siddharth Vadapalli wrote:
>>> Add the "cpsw-mac-efuse" node within "wkup_conf" node corresponding to the
>>> CTRLMMR_MAC_IDx registers within the CTRL_MMR space. Assign the compatible
>>> "ti,am62p-cpsw-mac-efuse" to enable "syscon_regmap" operations on these
>>> registers. The MAC Address programmed in the eFuse is accessible through
>>> the CTRLMMR_MAC_IDx registers. The "ti,syscon-efuse" device-tree property
>>> points to the CTRLMMR_MAC_IDx registers, allowing the CPSW driver to fetch
>>> the MAC Address and assign it to the network interface associated with
>>> CPSW3G MAC Port 1.
>>>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>>
>>> This patch is based on linux-next tagged next-20240404.
>>> Patch depends on:
>>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@ti.com/
>>> for the newly added "ti,am62p-cpsw-mac-efuse" compatible.
>>>
>>> v1:
>>> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402094200.4036076-1-s-vadapalli@ti.com/
>>> Changes since v1:
>>> - Since "wkup_conf" is modelled as a "simple-bus" rather than being
>>
>> And maybe the hardware representation is not correct? What bus is it?
> 
> I will let Andrew comment on it. Andrew had posted a patch at:
> https://lore.kernel.org/r/20240124184722.150615-10-afd@ti.com/
> to convert an equivalent "main_conf" node for AM62 SoC to "simple-bus"
> from the existing "syscon".
> 
>>
>>>   modelled as a System Controller node with the "syscon" compatible,
>>>   directly passing the reference to the "wkup_conf" node using the
>>>   "ti,syscon-efuse" device-tree property will not work.
>>>   Therefore, I posted the patch at:
>>>   https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@ti.com/
>>>   in order to add a new compatible to be used for modelling the
>>>   CTRLMMR_MAC_IDx registers as System Controller nodes, thereby
>>>   allowing the existing "ti,syscon-efuse" property to be used.
>>>   Now, "ti,syscon-efuse" points to the "cpsw_mac_efuse" node within
>>>   "wkup_conf" node, with "cpsw_mac_efuse" being a "syscon" node.
>>>
>>> Logs verifying that the CPSW driver assigns the MAC Address from the
>>> eFuse based on the CTRLMMR_MAC_IDx registers at 0x43000200 and 0x43000204
>>> to the interface eth0 corresponding to CPSW3G MAC Port 1:
>>> https://gist.github.com/Siddharth-Vadapalli-at-TI/9982c6f13bf9b8cfaf97e8517e7dea13
>>>
>>> Regards,
>>> Siddharth.
>>>
>>>  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi   | 1 +
>>>  arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 5 +++++
>>>  2 files changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> index 7337a9e13535..848ca454a411 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
>>> @@ -696,6 +696,7 @@ cpsw_port1: port@1 {
>>>  				label = "port1";
>>>  				phys = <&phy_gmii_sel 1>;
>>>  				mac-address = [00 00 00 00 00 00];
>>> +				ti,syscon-efuse = <&cpsw_mac_efuse 0x0>;
>>
>> Why this is not nvmem cell, like or efuses?
> 
> Since it belongs to the MMIO register set. You had recommended *not*
> using nvmem for such MMIO registers at:
> https://lore.kernel.org/r/48902771-5d3b-448a-8a74-ac18fb4f1a86@linaro.org/
> "nvmem is for non-volatile memory, like OCOTP and eFUSE. This is not for
> accessing regular MMIO registers of system-controller..."
> 
> Despite the "ti,syscon-efuse" property containing the term "efuse" in its
> name, it is reading the CTRLMMR_MAC_IDx MMIO registers. So I assumed that
> the existing approach which has been used on all K3 SoCs apart from this
> one, will be suitable for this SoC as well.

OK, I totally forgot we discussed this.

> 
>>
>>>  			};
>>>  
>>>  			cpsw_port2: port@2 {
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> index a84756c336d0..df9d40f64e3b 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
>>> @@ -18,6 +18,11 @@ chipid: chipid@14 {
>>>  			reg = <0x14 0x4>;
>>>  			bootph-all;
>>>  		};
>>> +
>>> +		cpsw_mac_efuse: cpsw-mac-efuse@200 {
>>
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> I was following the convention that other mfd-syscon compatible nodes
> seemed to be using:
> https://github.com/torvalds/linux/blob/41bccc98fb7931d63d03f326a746ac4d429c1dd3/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#L502
> The node is:
> dss_oldi_io_ctrl: dss-oldi-io-ctrl@41e0
> corresponding to the compatible:
> "ti,am654-dss-oldi-io-ctrl"
> which was added by commit:
> https://github.com/torvalds/linux/commit/cb523495ee2a5938fbdd30b8a35094d386c55c12

So if that one was wrong, then what? I don't know really what type of
device is it, but just because one contributor called it that way, does
not mean you should keep going. Maybe investigate why that contributor
did not decide to follow Devicetree spec recommendation?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: power: Add mediatek larb definition
From: Ulf Hansson @ 2024-04-04 10:02 UTC (permalink / raw)
  To: Yu-chang Lee (李禹璋)
  Cc: amergnat@baylibre.com, krzysztof.kozlowski@linaro.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	MandyJH Liu (劉人僖), conor+dt@kernel.org,
	Project_Global_Chrome_Upstream_Group, robh@kernel.org,
	Xiufeng Li (李秀峰),
	linux-arm-kernel@lists.infradead.org,
	krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
	Fan Chen (陳凡),
	angelogioacchino.delregno@collabora.com
In-Reply-To: <3b04c5344435cdb941b5d132e8f5fbfdf9188d67.camel@mediatek.com>

On Thu, 28 Mar 2024 at 07:06, Yu-chang Lee (李禹璋)
<Yu-chang.Lee@mediatek.com> wrote:
>
> On Wed, 2024-03-27 at 12:55 +0100, Alexandre Mergnat wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >  Hello Yu-chang Lee,
> >
> > SMI LARB must have a power domain, according to "mediatek,smi-
> > larb.yaml"
> > Now you try to create a link from power domain to larb.
> >
> > Here is my understanding: when you enable/disable power domain, the
> > larb linked to this power domain may have an issue. Then you want to
> > retrieve de LARB linked to the power domain though the dts to manage
> > the LARB.
>
> Yes, this is what I am trying to do.
>
> > IMHO, using the dts to have this information into the power
> > driver isn't necessary and may introduce some bugs if the LARB node
> > and power node in the DTS aren't aligned.
> >
> > It seems not implemented today but during the LARB probe, it should
> > "subscribe" to the linked power domain. Then, when the power domain
> > status is changing, it is able to "notify" (callback) the LARB, then
> > implement the good stuff to handle this power domain status change
> > into LARB driver.
> >
>
> The problem with this method and why "smi clamp" is in power domain
> driver is that our HW designer gave us a programming guide strictly
> states the sequence of what we need to do to power on/off power domain.
> Using callback, this sequence is no longer guaranteed and the side
> effect is unknown...

In most cases, using the runtime PM callbacks in the consumer driver
(LARB driver) is sufficient to deal with resets. For some special
cases drivers are making use of the genpd on/off notifiers
(GENPD_NOTIFY_*), as they really need to know when their devices have
been power collapsed. Have you tried both these options?

[...]

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers
From: Jagadeesh Kona @ 2024-04-04 10:06 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Vladimir Zapolskiy, linux-arm-msm, linux-clk, devicetree,
	linux-kernel, Taniya Das, Satya Priya Kakitapalli, Ajit Pandey,
	Imran Shaik
In-Reply-To: <CAA8EJprfaALkQe-wUrBow6B1A66ro0AoVpfnQJLXgqFmL8isNQ@mail.gmail.com>



On 4/4/2024 11:00 AM, Dmitry Baryshkov wrote:
> On Thu, 4 Apr 2024 at 08:13, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 4/3/2024 9:24 PM, Dmitry Baryshkov wrote:
>>> On Wed, 3 Apr 2024 at 10:16, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 3/25/2024 11:38 AM, Jagadeesh Kona wrote:
>>>>>
>>>>>
>>>>> On 3/21/2024 6:43 PM, Dmitry Baryshkov wrote:
>>>>>> On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Add device nodes for video and camera clock controllers on Qualcomm
>>>>>>> SM8650 platform.
>>>>>>>
>>>>>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>> ---
>>>>>>>     arch/arm64/boot/dts/qcom/sm8650.dtsi | 28 ++++++++++++++++++++++++++++
>>>>>>>     1 file changed, 28 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> index 32c0a7b9aded..d862aa6be824 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> @@ -4,6 +4,8 @@
>>>>>>>      */
>>>>>>>
>>>>>>>     #include <dt-bindings/clock/qcom,rpmh.h>
>>>>>>> +#include <dt-bindings/clock/qcom,sm8450-videocc.h>
>>>>>>> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>>>>>>>     #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>>>>>>>     #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>>>>>>>     #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>>>>>>> @@ -3110,6 +3112,32 @@ opp-202000000 {
>>>>>>>                            };
>>>>>>>                    };
>>>>>>>
>>>>>>> +               videocc: clock-controller@aaf0000 {
>>>>>>> +                       compatible = "qcom,sm8650-videocc";
>>>>>>> +                       reg = <0 0x0aaf0000 0 0x10000>;
>>>>>>> +                       clocks = <&bi_tcxo_div2>,
>>>>>>> +                                <&gcc GCC_VIDEO_AHB_CLK>;
>>>>>>> +                       power-domains = <&rpmhpd RPMHPD_MMCX>;
>>>>>>> +                       required-opps = <&rpmhpd_opp_low_svs>;
>>>>>>
>>>>>> The required-opps should no longer be necessary.
>>>>>>
>>>>>
>>>>> Sure, will check and remove this if not required.
>>>>
>>>>
>>>> I checked further on this and without required-opps, if there is no vote
>>>> on the power-domain & its peer from any other consumers, when runtime
>>>> get is called on device, it enables the power domain just at the minimum
>>>> non-zero level. But in some cases, the minimum non-zero level of
>>>> power-domain could be just retention and is not sufficient for clock
>>>> controller to operate, hence required-opps property is needed to specify
>>>> the minimum level required on power-domain for this clock controller.
>>>
>>> In which cases? If it ends up with the retention vote, it is a bug
>>> which must be fixed.
>>>
>>
>> The minimum non-zero level(configured from bootloaders) of MMCX is
>> retention on few chipsets but it can vary across the chipsets. Hence to
>> be on safer side from our end, it is good to have required-opps in DT to
>> specify the minimum level required for this clock controller.
> 
> We are discussing sm8650, not some abstract chipset. Does it list
> retention or low_svs as a minimal level for MMCX?
> 

Actually, the minimum level for MMCX is external to the clock 
controllers. But the clock controller requires MMCX to be atleast at 
lowsvs for it to be functional. Hence we need to keep required-opps to 
ensure the same without relying on the actual minimum level for MMCX.

Thanks,
Jagadeesh

>>>>>
>>>>>>> +                       #clock-cells = <1>;
>>>>>>> +                       #reset-cells = <1>;
>>>>>>> +                       #power-domain-cells = <1>;
>>>>>>> +               };
>>>>>>> +
>>>>>>> +               camcc: clock-controller@ade0000 {
>>>>>>> +                       compatible = "qcom,sm8650-camcc";
>>>>>>> +                       reg = <0 0x0ade0000 0 0x20000>;
>>>>>>> +                       clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>>>>>>> +                                <&bi_tcxo_div2>,
>>>>>>> +                                <&bi_tcxo_ao_div2>,
>>>>>>> +                                <&sleep_clk>;
>>>>>>> +                       power-domains = <&rpmhpd RPMHPD_MMCX>;
>>>>>>> +                       required-opps = <&rpmhpd_opp_low_svs>;
>>>>>>> +                       #clock-cells = <1>;
>>>>>>> +                       #reset-cells = <1>;
>>>>>>> +                       #power-domain-cells = <1>;
>>>>>>> +               };
>>>>>>> +
>>>>>>>                    mdss: display-subsystem@ae00000 {
>>>>>>>                            compatible = "qcom,sm8650-mdss";
>>>>>>>                            reg = <0 0x0ae00000 0 0x1000>;
>>>>>>> --
>>>>>>> 2.43.0
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>
>>>
>>>
> 
> 
> 

^ permalink raw reply

* Re: [PATCH v2] arm64: dts: ti: k3-am62p: use eFuse MAC Address for CPSW3G Port 1
From: Siddharth Vadapalli @ 2024-04-04 10:16 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Siddharth Vadapalli, afd, nm, vigneshr, kristo, robh, krzk+dt,
	conor+dt, devicetree, linux-kernel, linux-arm-kernel, srk
In-Reply-To: <903ad855-ab26-4ef3-80bd-249917056188@linaro.org>

On Thu, Apr 04, 2024 at 12:00:09PM +0200, Krzysztof Kozlowski wrote:
> On 04/04/2024 11:12, Siddharth Vadapalli wrote:
> > On Thu, Apr 04, 2024 at 10:43:04AM +0200, Krzysztof Kozlowski wrote:
> >> On 04/04/2024 10:18, Siddharth Vadapalli wrote:
> >>> Add the "cpsw-mac-efuse" node within "wkup_conf" node corresponding to the
> >>> CTRLMMR_MAC_IDx registers within the CTRL_MMR space. Assign the compatible
> >>> "ti,am62p-cpsw-mac-efuse" to enable "syscon_regmap" operations on these
> >>> registers. The MAC Address programmed in the eFuse is accessible through
> >>> the CTRLMMR_MAC_IDx registers. The "ti,syscon-efuse" device-tree property
> >>> points to the CTRLMMR_MAC_IDx registers, allowing the CPSW driver to fetch
> >>> the MAC Address and assign it to the network interface associated with
> >>> CPSW3G MAC Port 1.
> >>>
> >>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> >>> ---
> >>>

...

> 
> > 
> >>
> >>>  			};
> >>>  
> >>>  			cpsw_port2: port@2 {
> >>> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> >>> index a84756c336d0..df9d40f64e3b 100644
> >>> --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> >>> +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> >>> @@ -18,6 +18,11 @@ chipid: chipid@14 {
> >>>  			reg = <0x14 0x4>;
> >>>  			bootph-all;
> >>>  		};
> >>> +
> >>> +		cpsw_mac_efuse: cpsw-mac-efuse@200 {
> >>
> >> Node names should be generic. See also an explanation and list of
> >> examples (not exhaustive) in DT specification:
> >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > 
> > I was following the convention that other mfd-syscon compatible nodes
> > seemed to be using:
> > https://github.com/torvalds/linux/blob/41bccc98fb7931d63d03f326a746ac4d429c1dd3/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#L502
> > The node is:
> > dss_oldi_io_ctrl: dss-oldi-io-ctrl@41e0
> > corresponding to the compatible:
> > "ti,am654-dss-oldi-io-ctrl"
> > which was added by commit:
> > https://github.com/torvalds/linux/commit/cb523495ee2a5938fbdd30b8a35094d386c55c12
> 
> So if that one was wrong, then what? I don't know really what type of
> device is it, but just because one contributor called it that way, does
> not mean you should keep going. Maybe investigate why that contributor
> did not decide to follow Devicetree spec recommendation?

Yes, it doesn't justify the convention. I seem to have picked a wrong
example when figuring out the convention for naming the node. I plan to
name it as:
ethernet-mac-efuse
while retaining the label "cpsw_mac_efuse" since CPSW is the name of the
Ethernet Switch on the SoC. Please let me know if it is acceptable. I
will post the v3 patch based on your feedback.

Regards,
Siddharth.

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: iio: imu: add icm42688 inside inv_icm42600
From: Jean-Baptiste Maneyrol @ 2024-04-04 10:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, INV Git Commit, jic23@kernel.org,
	robh@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org
  Cc: lars@metafoo.de, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <beb68fc5-1ad8-4052-80c6-b706b62c267b@linaro.org>

Hello,

sorry about this mess, this is due to a problem in mail system configuration.

These patches are obviously not confidential. And IP from TDK-Micronas obviously is not applying to TDK-InvenSense products.

I will resend the patches when the issue is fixed.

Sorry for the inconvenience.

Best regards,
JB

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Sent: Thursday, April 4, 2024 09:03
To: INV Git Commit <INV.git-commit@tdk.com>; jic23@kernel.org <jic23@kernel.org>; robh@kernel.org <robh@kernel.org>; krzysztof.kozlowski+dt@linaro.org <krzysztof.kozlowski+dt@linaro.org>; conor+dt@kernel.org <conor+dt@kernel.org>
Cc: lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: imu: add icm42688 inside inv_icm42600
 
This Message Is From an External Sender
This message came from outside your organization.
 
On 02/04/2024 11:00, inv.git-commit@tdk.com wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Add bindings for ICM-42688-P chip.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> ---
>  .../devicetree/bindings/iio/imu/invensense,icm42600.yaml         | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 7cd05bcbee31..5e0bed2c45de 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -32,6 +32,7 @@ properties:
>        - invensense,icm42605
>        - invensense,icm42622
>        - invensense,icm42631
> +      - invensense,icm42688
> 
>    reg:
>      maxItems: 1
> --
> 2.34.1
> 
> TDK-Micronas GmbH
> Company Headquarters / Sitz der Gesellschaft: Freiburg i. Br. - Municipal Court of / Amtsgericht: Freiburg i. Br. HRB 6108. VAT ID / USt-IdNr.: DE 812878184
> Management / Geschäftsführung: Sam Maddalena
> 
> This e-mail and any files transmitted with it are confidential information of TDK-Micronas and intended solely for the use of the individual or entity to whom they are addressed. If you have received this e-mail in error please notify the sender by return e-mail and delete all copies of this e-mail message along with all attachments. If you are not the named addressee you should not disseminate, distribute or copy this e-mail.

Don't send patches which are confidential. Community cannot take them.

Best regards,
Krzysztof


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox