[PATCH] aspeed/g5: Device Tree for ast2500 and eval board, I2C driver

[Maxim Sloyko]

Hi Joel,

Sorry for the delay with the reply, my work computer stopped working,
needed to attend that.

I split the patch into three different self-consistent (hopefully) patches,
but somewhat different from what you've suggested.

On Thu, Sep 22, 2016 at 6:59 PM, Joel Stanley <joel at jms.id.au> wrote:

> Hello Maxim,
>
> On Tue, Sep 20, 2016 at 2:56 AM,  <maxims at google.com> wrote:
> > From: Maxim Sloyko <maxims at google.com>
> >
> > The driver is limited: only single master mode is supported and only
> > byte-by-byte reads and writes are supported, no DMA or Pool Buffers.
>
> Has this code been ported from the kernel driver? If so, mention it in
> your commit message and/or a comment in the code. A comment about what
> needed to be modified in order to make it integrate in the u-boot tree
> would be helpful. Also mention where you copied the code from; which
> tree or commit.
>

I would not call that "copied". The APIs are very different at the moment,
especially since in u-boot device tree is not fully utilized, so yes, I
used kernel driver as a reference, but this is essentially a rewrite.


>
> Be sure to incorporate any changes made to the upstream Linux driver
> as it goes through review.
>
> > Also, pin function configuration is performed by the I2C driver, because
> > there is no pinctrl driver at the moment.
>
> It looks like you've got several patches combined into one. To make it
> easier to review, try to have a separate patch for each feature you're
> adding.
>
> > ---
> >  arch/arm/dts/Makefile                       |   2 +
> >  arch/arm/dts/aspeed-g5-evb.dts              |  28 ++
> >  arch/arm/dts/aspeed-g5.dtsi                 | 392
> ++++++++++++++++++++++++++++
>
> This would be one patch.
>
> I assume they came from the kernel tree? If so, mention this,
> including which tree/commit you got them from.
>
> >  arch/arm/include/asm/arch-aspeed/ast_scu.h  |   6 +
> >  arch/arm/include/asm/arch-aspeed/regs-scu.h |  74 +++---
>
> This would be another.
>
> >  arch/arm/mach-aspeed/ast-scu.c              |  31 ++-
> >  board/aspeed/ast-g5/ast-g5.c                |   2 +-
>
> I believe these integrate the i2c driver with the arch and the board.
> They should be separate commits.
>
> >  configs/ast_g5_defconfig                    |   4 +-
>
> Updating the defconfig should be it's own patch.
>

This change was dropped.


>
> >  drivers/i2c/Kconfig                         |   6 +
> >  drivers/i2c/Makefile                        |   1 +
> >  drivers/i2c/ast_i2c.c                       | 306 ++++++++++++++++++++++
> >  drivers/i2c/ast_i2c.h                       | 155 +++++++++++
>
> Adding the i2c driver has nothing to do with the other functions, so
> that could be it's own patch.
>
> Cheers,
>
> Joel
>



-- 
*M*axim *S*loyko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160927/90128308/attachment-0001.html>