Re: [PATCH v2 1/2] ALSA: docs: Add Marian M2 driver documentation

[Ivan Orlov]

On 9/19/23 11:57, Jonathan Corbet wrote:
> Ivan Orlov <ivan.orlov0322@xxxxxxxxx> writes:
>
> > Add documentation for the new MARIAN Seraph M2 sound card. It covers
> > current development status, available controls of the card and
> > information about the integrated loopback.
> >
> > Signed-off-by: Ivan Orlov <ivan.orlov0322@xxxxxxxxx>
> > ---
> > V1 -> V2:
> > - Remove redundant documentation fix from the next patch in the series
>
> One quick comment below...
>
> >   Documentation/sound/cards/index.rst     |   1 +
> >   Documentation/sound/cards/marian-m2.rst | 104 ++++++++++++++++++++++++
> >   2 files changed, 105 insertions(+)
> >   create mode 100644 Documentation/sound/cards/marian-m2.rst
> >
> > diff --git a/Documentation/sound/cards/index.rst b/Documentation/sound/cards/index.rst
> > index e68bbb13c384..e873592d8d00 100644
> > --- a/Documentation/sound/cards/index.rst
> > +++ b/Documentation/sound/cards/index.rst
> > @@ -19,3 +19,4 @@ Card-Specific Information
> >      serial-u16550
> >      img-spdif-in
> >      pcmtest
> > +   marian-m2
> > diff --git a/Documentation/sound/cards/marian-m2.rst b/Documentation/sound/cards/marian-m2.rst
> > new file mode 100644
> > index 000000000000..bf12445e20d7
> > --- /dev/null
> > +++ b/Documentation/sound/cards/marian-m2.rst
> > @@ -0,0 +1,104 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +=======================
> > +MARIAN Seraph M2 Driver
> > +=======================
> > +
> > +Sep 18, 2023
> > +
> > +Ivan Orlov <ivan.orlov0322@xxxxxxxxx>
> > +
> > +STATE OF DEVELOPMENT
> > +====================
> > +
> > +This driver is based on the driver written by Florian Faber in 2012, which seemed to work fine.
> > +However, the initial code contained multiple issues, which had to be solved before sending the
> > +driver upstream.
>
> Sticking to the 80-column limit is best for documentation, especially
> when there is no reason to go over it.
>
> > +The vendor lost the full documentation, so what we have here was recovered from drafts and found
> > +after experiments with the card.
> > +
> > +What seems to be working fine:
> > +- Playback and capture for all supported rates
> > +- Integrated loopback (with some exceptions, see below)
> > +
> > +MEMORY MODEL
> > +============
> > +
> > +The hardware requires one huge contiguous DMA space to be allocated. After allocation, the bus address of
> > +this buffer should be written to the hardware register.
> > +
> > +We can split this space into two parts: the first one contains samples for capture, another one contains
> > +play samples:
> > +
> > +CAPTURE_CH_0, CAPTURE_CH_1, ..., CAPTURE_CH_127 | PLAY_CH_0, PLAY_CH_1, ..., PLAY_CH_127
>
> You should really use literal blocks for this (and a lot that follows)
> or it won't render the way you want in HTML.  The simplest way to do
> that is to use a double colon ("...samples::") and to indent the literal
> text.
>
> (OK, two comments, sorry about the off-by-one...:)
>
> Thanks,
>
> jon

Hi Jonathan,

Thank you for the review, I'll rewrite the documentation so it will 
follow the 80-column rule. Also, in the next version I will use the 
literal blocks.

Thanks again!

--
Kind regards,
Ivan Orlov