Re: [PATCH] Fixed the schema binding according to test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux