From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mingming Lee <mingming.lee@mediatek.com>
Date: Wed, 30 Sep 2020 14:15:47 +0800
Subject: [PATCH 1/3] i2c: mediatek: add basic driver support
In-Reply-To: <22a371ad-76d6-ff6d-ded3-2eeea84bb5ab@denx.de>
References: <20200909060728.5616-1-mingming.lee@mediatek.com>
 <20200909060728.5616-2-mingming.lee@mediatek.com>
 <22a371ad-76d6-ff6d-ded3-2eeea84bb5ab@denx.de>
Message-ID: <1601446547.9215.3.camel@mhfsdcap03>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Wed, 2020-09-30 at 07:38 +0200, Heiko Schocher wrote:
> Hello mingming,
> 
> Am 09.09.2020 um 08:07 schrieb mingming lee:
> > From: Mingming Lee <Mingming.Lee@mediatek.com>
> > 
> > Add MediaTek I2C basic driver
> > 
> > Signed-off-by: Mingming Lee <Mingming.Lee@mediatek.com>
> > ---
> >   drivers/i2c/Kconfig  |   9 +
> >   drivers/i2c/Makefile |   1 +
> >   drivers/i2c/mt_i2c.c | 619 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 629 insertions(+)
> >   create mode 100644 drivers/i2c/mt_i2c.c
> 
> Nitpicks only:
> 
> As you introduce here new DTS bindings, can you please document them
> in u-boot:/doc/device-tree-bindings/
> 
----->thank you for for advice, I would add the new DTS bingding in next
version.

> Thanks!
> 
> 
> [snip]
> > diff --git a/drivers/i2c/mt_i2c.c b/drivers/i2c/mt_i2c.c
> > new file mode 100644
> > index 0000000..5badde9
> > --- /dev/null
> > +++ b/drivers/i2c/mt_i2c.c
> > @@ -0,0 +1,619 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * MediaTek I2C Interface driver
> > + *
> > + * Copyright (C) 2020 MediaTek Inc.
> > + * Author: Mingming Lee <Mingming.Lee@mediatek.com>
> > + */
> > +
> > +#include <asm/cache.h>
> > +#include <asm/io.h>
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <i2c.h>
> > +#include <linux/delay.h>
> > +
> > +#define I2C_OK			0
> > +#define EINVAL_I2C		22  /* invalid argument */
> > +#define ETIMEDOUT_I2C		110 /* connection timed out */
> > +#define EACK_I2C		6   /* remote I/O error */
> 
> You could use defines from include/linux/errno.h ?
> 
------->I think it is better to use the define from
include/linux/errno.h, and I would modify it in next version today.

> [snip]
> 
> 
> Reviewed-by: Heiko Schocher <hs@denx.de>
> 
> Thanks!
> 
> bye,
> Heiko