[PATCH v2 1/2] i2c: aspeed: added driver for Aspeed I2C

[Brendan Higgins]

> Can you update this to 2016? And add your own?

Done.

> I would prefer using the ASPEED_ prefix for all #defines and functions
> for consistency with other kernel code.

Done.

> Typo: Global

Done.

> If you want to convert these to use the BIT(1), etc notation.

Done.

> Can we get away without the guard here?

Done.

> s/sts/status/

Actually this is the command register, but I fixed it.

> Would goto out, with out being just above the _ireqrestore at the
> bottom of this function, make it clearer that we are doing the correct
> lock/unlocking?

I was on the fence about this, so your comment gave me reason enough to fix it.

> I assume we are releasing the lock to wait for the completion?

Yep, I do not think a comment is appropriate here: it is pretty clear that the
lock is will be held by the IRQ handler.

> Above we call it "sts", down here we call it "irq_status". I suggest a
> consistent name.

I renamed the above to "command", so this is fixed.

> What is "value"? Perhaps "request"?

So this is actually the value that gets passed in/out of the slave callback; it
is just an arbitrary byte. I agree that the name is not *great*, but I could not
think of a better one (could be an in or out). This does follow the convention
set by Wolfram in his i2c-slave-eeprom driver which is the example driver for
the slave calback framework (although he used val, I like value better than
val).

> You're testing for the same thing in both if loops. Could you
> rearrange the logic?

Done.

> As above.

Done.

> Not sure about this. Are we able to leave them as null?

Nope. I tried it.

> Could we just create the buses that are defined as enabled in the device tree?

Done.

> Drop this. We've rewritten it since we took Ryan's code from the BSP.

Done.

> Not sure about this? Keep it if you feel comfortable answering
> questions about the design of the entire driver.

I don't mind. I expect to clean up after my own mistakes.

> Aspeed I2C Bus driver?

Done.

> Not sure that we need this.

Done.

--------------------------------------------------------------------------------

Changes:
  - Addressed comments above.
  - Found and fixed bug in error reporting (did not clear .cmd_err).
  - Found and fixed bug in unreg_slave (did not clean up after self properly).

Cheers