[PATCH] Fixed the schema binding according to test
Ki-Seok Jo
kiseok.jo at irondevice.com
Thu Feb 2 10:55:47 CET 2023
> 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 at 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 at 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
More information about the Alsa-devel
mailing list