[PATCH 3/3] arm64: dts: rockchip: add Anbernic RG353 and RG503

[Krzysztof Kozlowski]

On 18/08/2022 19:33, Chris Morgan wrote:
> On Thu, Aug 18, 2022 at 11:14:17AM +0300, Krzysztof Kozlowski wrote:
>> On 17/08/2022 23:49, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan at hotmail.com>
>>>
>>> Anbernic RG353 and RG503 are both RK3566 based handheld gaming devices
>>> from Anbernic.
>>>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> +		red_led: led-2 {
>>> +			color = <LED_COLOR_ID_RED>;
>>> +			default-state = "off";
>>> +			function = LED_FUNCTION_STATUS;
>>> +			gpios = <&gpio0 RK_PC7 GPIO_ACTIVE_HIGH>;
>>> +		};
>>> +	};
>>> +
>>> +	rk817-sound {
>>
>> just sound
>>
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
> 
> Acknowledged. I just cut and pasted from a different tree, but I'll make this change.
> 
>>> +		compatible = "simple-audio-card";
>>> +		simple-audio-card,name = "anbernic_rk817";
>>> +		simple-audio-card,aux-devs = <&spk_amp>;
>>> +		simple-audio-card,format = "i2s";
>>> +		simple-audio-card,hp-det-gpio = <&gpio4 RK_PC6 GPIO_ACTIVE_HIGH>;
>>> +		simple-audio-card,mclk-fs = <256>;
>>> +		simple-audio-card,widgets =
>>> +			"Microphone", "Mic Jack",
>>> +			"Headphone", "Headphones",
>>> +			"Speaker", "Internal Speakers";
>>> +		simple-audio-card,routing =
>>> +			"MICL", "Mic Jack",
>>> +			"Headphones", "HPOL",
>>> +			"Headphones", "HPOR",
>>> +			"Internal Speakers", "Speaker Amp OUTL",
>>> +			"Internal Speakers", "Speaker Amp OUTR",
>>> +			"Speaker Amp INL", "HPOL",
>>> +			"Speaker Amp INR", "HPOR";
>>> +		simple-audio-card,pin-switches = "Internal Speakers";
>>> +
>>> +		simple-audio-card,codec {
>>> +			sound-dai = <&rk817>;
>>> +		};
>>> +
>>> +		simple-audio-card,cpu {
>>> +			sound-dai = <&i2s1_8ch>;
>>> +		};
>>> +	};
>>> +
>>> +	sdio_pwrseq: sdio-pwrseq {
>>> +		compatible = "mmc-pwrseq-simple";
>>> +		clocks = <&rk817 1>;
>>> +		clock-names = "ext_clock";
>>> +		pinctrl-0 = <&wifi_enable_h>;
>>> +		pinctrl-names = "default";
>>> +		post-power-on-delay-ms = <200>;
>>> +		reset-gpios = <&gpio4 RK_PA2 GPIO_ACTIVE_LOW>;
>>> +	};
>>> +
>>> +	spk_amp: audio-amplifier {
>>> +		compatible = "simple-audio-amplifier";
>>> +		enable-gpios = <&gpio4 RK_PC2 GPIO_ACTIVE_HIGH>;
>>> +		pinctrl-0 = <&spk_amp_enable_h>;
>>> +		pinctrl-names = "default";
>>> +		sound-name-prefix = "Speaker Amp";
>>> +	};
>>> +
>>> +	vcc3v3_lcd0_n: vcc3v3-lcd0-n {
>>
>> Node name:
>> regulator-vcc3v3-lcd0-n
>> vcc3v3-lcd0-n-regulator
>> or just regulator-0
> 
> Does this restriction only apply to node names for regulators, or all
> node names? The docs I looked at suggested that it was okay to use an
> underscore, but I'll defer to you.

Device node names should be generic and such rule applies everywhere.
For regulators and clocks, pretty often people want some specific
prefix/suffix, so I don't mind, but what I mind is the generic part.

underscores will get you warnings with W=1, so they are not accepted
even though the spec mentions them.

Best regards,
Krzysztof