Re: [PATCH 02/13] dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example

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

 



On 19/02/2024 14:41, Théo Lebrun wrote:
> Hello,
> 
> On Sat Feb 17, 2024 at 9:25 AM CET, Krzysztof Kozlowski wrote:
>> On 16/02/2024 11:40, Théo Lebrun wrote:
>>> On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote:
>>>> On 16/02/2024 11:18, Théo Lebrun wrote:
>>>>>
>>>>>>> +        mobileye,id:
>>>>>>> +          $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> +          description: Platform-wide controller ID (integer starting from zero).
>>>>>>
>>>>>> instance indexes are a NAK. You can use i2cN aliases if you must.
>>>>>>
>>>>>> Why do you need it? To access OLB? If so, add cell args to the OLB 
>>>>>> phandle instead.
>>>>>
>>>>> Why we do what we do: I2C controller must write a 2 bit value depending
>>>>> on the bus speed. All I2C controllers write into the same register.
>>>>
>>>> Which register?  Your devices do not share IO address space.
>>>
>>> mobileye,olb is a prop with a phandle to a syscon. That syscon contains
>>> the register we are interested in.
>>
>> So exactly what Rob said... I don't understand why you have chosen to go
>> with alias.
> 
> I had misunderstood Rob's original message. Now that I've done some
> tests to use cells I get what was meant. I'd have a follow-up question.
> What should the cells contain? I see two options:
> 
>  - phandle + I2C controller global index (from 0 thru 4). Then Linux
>    (or other) driver know how to map that index to register + mask
>    combo. ie:
> 
>       i2c2: i2c@500000 {
>          compatible = "mobileye,eyeq5-i2c", "arm,primecell";
>          reg = <0 0x500000 0x0 0x1000>;
>          /* ... */
>          mobileye,olb = <&olb 2>;
>       };
> 
>  - phandle + register offset + mask. ie:
> 
>       i2c2: i2c@500000 {
>          compatible = "mobileye,eyeq5-i2c", "arm,primecell";
>          reg = <0 0x500000 0x0 0x1000>;
>          /* ... */
>          mobileye,olb = <&olb 0xB8 0x300>; /* phandle + offset + mask */
>       };

Whichever works for your current and possibly future needs and hardware,
because property should have one meaning. It's anyway specific to the
property. Second option is quite popular. Please design it for entire
hardware, not for this one particular case.

Best regards,
Krzysztof





[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux