[PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver

Doug Anderson dianders at chromium.org
Wed Apr 17 02:25:26 EST 2013


Wolfram / Guenter / Stephen,

Thanks for your reviews.  New version on its way...

On Tue, Apr 16, 2013 at 2:36 AM, Wolfram Sang <wsa at the-dreams.de> wrote:
> "uses GPIO lines and a challange & response mechanism" or something like
> that. There might be other GPIO based arbitrations in the future (though
> I hope there won't).

Done (including fixing spelling of challenge).  Stephen: I think the
new paragraph is still high-level enough even with Wolfram's requested
addition.


>> +- their-claim-gpios: The GPIOs that the other sides use the claim the bus.
>> +  Note that some implementations may only support a single other master.
>
> Stronger? "Currently, only one other master is supported"?

As per Stephen Warren, I didn't touch this.  He says:

The DT binding documentation, which should be OS-/driver-agnostic,
should describe the binding, not the implementation. The limitation that
Linux imposes is OS-specific and hence should not be mentioned here as
an absolute, or perhaps even at all.

I think the current wording about "may only support a single other
master" is a compromise and would at least give some a clue that they
should check the Linux driver for details.


>> +       If you say yes to this option, support will be included for an
>> +       I2C multimaster arbitration scheme using GPIOs where masters have
>> +       to claim the bus by asserting a GPIO.
>
> "callenge & response"?

Done (including fixing spelling of challenge).


>> + * GPIO-based I2C Arbitration
>
> "callenge & response"?

Done (including fixing spelling of challenge).


>> +struct i2c_arbitrator_data {
>> +     struct i2c_adapter *parent;
>> +     struct i2c_adapter *child;
>> +
>
> No empty line.

Done.


>> +     int             our_gpio;
>> +     int             our_gpio_release;
>> +     int             their_gpio;
>> +     int             their_gpio_release;
>> +     unsigned int    slew_delay_us;
>> +     unsigned int    wait_retry_us;
>> +     unsigned int    wait_free_us;
>> +};
>
> Single space as indentaion after "int".

Done.


More information about the devicetree-discuss mailing list