Thank you for your patch. There is something to discuss/improve.
On 02/02/2023 10:07, Kiseok Jo wrote:
Modified according to the writing-schema.rst file and tested.
Use imperative, not past tense (Fixed->Fix, Modified->Modify).
Okay. I got it. I'll do that when I rewrite it. Thanks.
Signed-off-by: Kiseok Jo kiseok.jo@irondevice.com
Use subject prefixes matching the subsystem (which you can get for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching). Therefore it should be: "ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties"
Oh, thank you for good information. I feel like I still lack a lot. I'll apply it. Thanks!
.../bindings/sound/irondevice,sma1303.yaml | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml index 162c52606635..35d9a046ef75 100644 --- a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml +++ b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml @@ -10,22 +10,62 @@ maintainers:
- Kiseok Jo kiseok.jo@irondevice.com
description:
- SMA1303 digital class-D audio amplifier with an integrated boost
converter.
- SMA1303 digital class-D audio amplifier with an integrated boost
- converter.
allOf:
- $ref: name-prefix.yaml#
- $ref: dai-common.yaml#
+properties:
- compatible:
- enum:
- irondevice,sma1303- reg:
- maxItems: 1
- '#sound-dai-cells':
- const: 1
- i2c-retry:
- description: number of retries for I2C regmap.
Why do you need this? Why this fits the purpose of DT (or IOW why this differs between boards)?
When working with drivers on mulitiple platforms, there were cases where I2C did not work properly dpending on the AP or setting. So I made it possible to set a few retry settings, and then check or do other actions. Retry is performed only when I2C fails.
And each device may have a different pull-up resisor or strength, so there may be differences between boards.
Could that property be a problem?
- maximum: 49
- default: 3
- tdm-slot-rx:
- description: set the tdm rx start slot.
Aren't you now re-writing dai-tdm-slot-rx-mask property? Same for tx below.
It can be the same as audio DAI's tdm slot, I think but there are cases where it is set differently, so I omitted it separately.
- maximum: 7
- default: 0
- tdm-slot-tx:
- description: set the tdm tx start slot.
- maximum: 7
- default: 0
- sys-clk-id:
- description: select the using system clock.
What does it mean? Why do you need such property instead of clocks?
This can receive an external clock, but it can use internal clock. Should I write all the clock descriptions in case?
- default: 3
required:
- compatible
- reg
- '#sound-dai-cells'
Best regards, Krzysztof
Thank you for quick response! I'll take a look to see if there are any other issues.
Thanks.
Best regards, Kiseok Jo