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@xxxxxxxxxxx>
>>>
>>> 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
