Re: [PATCH 3/6] dt/bindings: Add bindings for Tegra GMI controller

[Jon Hunter <jonathanh@nvidia.com>]

On 10/08/16 09:45, Jon Hunter wrote:
> 
> On 09/08/16 21:48, Mirza Krak wrote:
>> 2016-08-09 15:34 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>>>
>>> On 09/08/16 09:40, Mirza Krak wrote:
>>>> 2016-08-08 16:44 GMT+02:00 Jon Hunter <jonathanh@nvidia.com>:
>>>>>
>>>>> On 06/08/16 20:40, Mirza Krak wrote:
>>>>>> From: Mirza Krak <mirza.krak@gmail.com>
>>>>>>
>>
>> <--snip-->
>>
>>>>>> + - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
>>>>>> +   Valid values are 0-15, default is 1
>>>>>> + - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
>>>>>> +   Valid values are 0-255, default is 1
>>>>>> + - nvidia,snor-wait-width: Number of cycles before READY is asserted.
>>>>>> +   Valid values are 0-255, default is 3
>>>>>> +
>>>>>> +Example with two SJA1000 CAN controllers connected to the GMI bus:
>>>>>> +
>>>>>> +  gmi@70090000 {
>>>>>> +    #address-cells = <1>;
>>>>>> +    #size-cells = <1>;
>>>>>
>>>>> I think 0 for size makes sense. I know that caused you problems before,
>>>>> but I am wondering if ...
>>>>>
>>>>>> +    ranges;
>>>>>
>>>>> ... ranges is needed here? If we do have it, I am wondering if it should
>>>>> be a single entry for the chip-select that is being used. For now we
>>>>> could only support a ranges with one entry.
>>>>>
>>>>>         #address-cells = <1>;
>>>>>         #size-cells = <1>;
>>>>>         ranges = <4 0x48000000 0x00040000>;
>>>>
>>>> I prefer if we have "ranges" with one single entry, and warn if user enters
>>>> multiple for now, like we discussed earlier. Should have really done it in
>>>> this series.
>>>
>>> I think I do as well.
>>>
>>>>>
>>>>>> +    nvidia,snor-mux-mode;
>>>>>> +    nvidia,snor-adv-inv;
>>>>>> +    nvidia,snor-cs-select = <4>;
>>>>>
>>>>> I would have expected these under bus@X node as they are specific to the
>>>>> GMI CS.
>>>>
>>>> Yes, that is true.
>>>>
>>>>>
>>>>> I would also expect that the actual chip-select number is encoded in the
>>>>> reg property.
>>>>>
>>>>>> +
>>>>>> +    bus@0,0 {
>>>>>
>>>>> bus@4
>>>>
>>>> ACK.
>>>>
>>>>>
>>>>> No mention of this bus node in the above documentation.
>>>>
>>>> I was hesitant documenting it since I am not sure if we really need it
>>>> in a generic case? It does make sense in my
>>>> specific case. But what would it look like if we could maintain CS in software.
>>>>
>>>> Do you then have a bus node per CS? I am guessing not. Is it enough to
>>>> document it in my "example brief"?
>>>
>>> I see what you mean. May be it is fine to document with the examples
>>> below how child devices should be populated. You should also state that
>>> currently the GMI only supports one child device currently for the
>>> chip-select that is being used.
>>
>> Should I really need to state that? Is it not always the case, that
>> you have one child device per chip-select?
> 
> I think so. Remember it is one child device for the GMI, however, that
> child device could still have one than one child device (per you example

s/one than one/more than one

Jon

-- 
nvpublic