Re: [PATCH v8 01/13] Documentation: Add SLIMbus summary

[Srinivas Kandagatla]

Thanks for the review comments.

On 01/12/17 10:27, Jonathan NeuschÃfer wrote:
> Hi, some small nits below.
>
> On Thu, Nov 30, 2017 at 05:41:48PM +0000, srinivas.kandagatla@xxxxxxxxxx wrote:
> > From: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
> >
> > SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> > developed by MIPI (Mobile Industry Processor Interface) alliance.
> > SLIMbus is a 2-wire implementation, which is used to communicate with
> > peripheral components like audio-codec.
> >
> > The summary of SLIMbus and API is documented in the 'summary' file.
> >
> > Signed-off-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > ---
> >   Documentation/driver-api/slimbus/index.rst   |  15 ++++
> >   Documentation/driver-api/slimbus/summary.rst | 108 +++++++++++++++++++++++++++
> >   2 files changed, 123 insertions(+)
> >   create mode 100644 Documentation/driver-api/slimbus/index.rst
> >   create mode 100644 Documentation/driver-api/slimbus/summary.rst
>
> It would probably make sense to Cc the documentation maintainers/mailing
> list on this patch (AFAICS, you didn't do that).
Yep, I will include them while sending next version.

> When do you plan to add slimbus to Documentation/driver-api/index.rst?

that's good point, I will fix this in next version.
>

> > +framework allows drivers to bind when corresponding devices report their
> > +presence on the bus.
> > +However, it is possible that the driver needs to be probed
> > +first so that it can enable corresponding SLIMbus device (e.g. power it up and/or
> > +take it out of reset). To support that behavior, the framework allows drivers
> > +to probe first as well  (e.g. using standard DeviceTree compatbility field).
>
> Typo: s/compatbility/compatibility/
Yep, Will fix in next version.

> > +---------------
> > +The framework supports APIs to exchange control-information with a SLIMbus
> > +device. APIs can be synchronous or asynchronous.
> > +From controller's perspective, multiple buffers can be queued to/from
> > +hardware for sending/receiving data using slim_ctrl_buf circular buffer.
> > +The header file <linux/slimbus.h> has more documentation about messaging APIs.
>
> Once the kerneldoc documentation (i.e. the /** ... */ comments in the
> source) is included somewhere, I think it would make sense to make
> slim_ctrl_buf a clickable link to the struct's documentation.
Sure, I will explore this side of it before i send next version.

thanks,
srini
> Thanks,
> Jonathan NeuschÃfer