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