Subject: Re: [PATCH v8 01/13] Documentation: Add SLIMbus summary
To: =?UTF-8?Q?Jonathan_Neusch=c3=a4fer?= <j.neuschaefer@gmx.net>
Cc: gregkh@linuxfoundation.org, broonie@kernel.org,
        alsa-devel@alsa-project.org, sdharia@codeaurora.org, bp@suse.de,
        poeschel@lemonage.de, treding@nvidia.com, andreas.noever@gmail.com,
        alan@linux.intel.com, mathieu.poirier@linaro.org, daniel@ffwll.ch,
        jkosina@suse.cz, sharon.dvir1@mail.huji.ac.il, joe@perches.com,
        davem@davemloft.net, james.hogan@imgtec.com,
        michael.opdenacker@free-electrons.com, robh+dt@kernel.org,
        pawel.moll@arm.com, mark.rutland@arm.com, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        vinod.koul@intel.com, arnd@arndb.de
References: <20171130174200.6684-1-srinivas.kandagatla@linaro.org>
 <20171130174200.6684-2-srinivas.kandagatla@linaro.org>
 <20171201102758.bcziygl3k46iepc6@latitude>
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Message-ID: <96cbc048-e76a-ad36-bba2-e717bc8bc05b@linaro.org>
Date: Tue, 5 Dec 2017 22:01:50 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20171201102758.bcziygl3k46iepc6@latitude>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2552
Lines: 64

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@linaro.org wrote:
>> From: Sagar Dharia <sdharia@codeaurora.org>
>>
>> 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@codeaurora.org>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   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
>