Hi Sergei,
On Thu, 8 Dec 2022 at 06:09, Sergei Antonov <sap...@gmail.com> wrote:
>
> On Wed, 7 Dec 2022 at 04:08, Simon Glass <s...@chromium.org> wrote:
> >
> > Hi Sergei,
> >
> > On Tue, 6 Dec 2022 at 23:07, Sergei Antonov <sap...@gmail.com> wrote:
> > >
> > > Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
> > > , minutes, hours, day of the week, date, month and year information.
> > >
> > > Datasheet:
> > > https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
> > >
> > > Signed-off-by: Sergei Antonov <sap...@gmail.com>
> > > ---
> > >
> > > v2:
> > > * The RESET pin is now to be described as ACTIVE_LOW in dts.
> > >
> > > Changes suggested by Simon Glass:
> > > * a more detailed driver description in Kconfig
> > > * multi-line comments' style
> > > * enum for 0x80 and the 0x20 at top of file
> > > * lower-case hex constants
> > > * function comments for ht1380_reset_on/off
> > > * blank line before returns
> > >
> > > PROTECT remains in a function scope for the sake of locality of
> > > definitions.
> > >
> > > drivers/rtc/Kconfig | 8 +
> > > drivers/rtc/Makefile | 1 +
> > > drivers/rtc/ht1380.c | 337 +++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 346 insertions(+)
> > > create mode 100644 drivers/rtc/ht1380.c
> > >
[..]
> > > +static int ht1380_rtc_get(struct udevice *dev, struct rtc_time *tm)
> > > +{
> > > + struct ht1380_priv *priv = dev_get_priv(dev);
> > > + int ret, i, bit, reg[N_REGS];
> > > +
> > > + ret = dm_gpio_set_value(&priv->clk_desc, 0);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = dm_gpio_set_dir_flags(&priv->dat_desc, GPIOD_IS_OUT);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = ht1380_reset_off(priv);
> > > + if (ret)
> > > + goto exit;
> > > +
> > > + ret = ht1380_send_byte(priv, BURST + READ);
> > > + if (ret)
> > > + goto exit;
> > > +
> > > + ret = dm_gpio_set_dir_flags(&priv->dat_desc, GPIOD_IS_IN);
> > > + if (ret)
> > > + goto exit;
> > > +
> >
> > Is this some sort of I2C protocol?
>
> Like I2C it uses a pin for clock and a pin for data in/out. Unlike I2C
> it does not use addressing. I am not sure whether this driver can
> utilize some of the existing I2C code in U-Boot. Wrote my own bit
> banging routines.
Yes you can use i2c, by setting the offset_len to 0, e.g. with:
u-boot,i2c-offset-len = <0>;
>
> > > + for (i = 0; i < N_REGS; i++) {
> > > + reg[i] = 0;
> > > +
> > > + for (bit = 0; bit < 8; bit++) {
> > > + ht1380_half_period_delay();
> > > +
> > > + ret = dm_gpio_set_value(&priv->clk_desc, 1);
> > > + if (ret)
> > > + goto exit;
> > > + ht1380_half_period_delay();
> > > +
> > > + reg[i] |= dm_gpio_get_value(&priv->dat_desc) <<
> > > bit;
> > > + ret = dm_gpio_set_value(&priv->clk_desc, 0);
> > > + if (ret)
> > > + goto exit;
> > > + }
> > > + }
> > > +
> > > + ret = -EINVAL;
> > > +
> > > + /* Correctness check: some bits are always zero */
> > > + if ((reg[MIN] & 0x80) || (reg[HOUR] & 0x40) || (reg[MDAY] & 0xc0)
> > > ||
> > > + (reg[MONTH] & 0xe0) || (reg[WDAY] & 0xf8) || (reg[WP] & 0x7f))
> > > + goto exit;
> >
> > Drop extra brackets ?
>
> OK. Will be done in v3. I put extra brackets expecting reviewers to
> criticize code readability :).
:)
Regards,
Simon
