On Thu, Sep 19, 2024 at 2:36 AM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Wed, 18 Sept 2024 at 22:31, Corey Minyard <co...@minyard.net> wrote:
> > Generally it's frowned upon to have a ton of extra stuff that's not
> > used. I would think some is ok, like defining bits in registers that
> > aren't used yet, but I have no idea how all the enums below here
> > actually tie into anything. I'm guessing these are just bitmasks into
> > registers, but really, it's a lot easier to read if you have something
> > like:
> >
> > /*
> > * The I2C Master function enable. When disabled, the Master
> > * configuration settings are not changed, but the Master function is
> > * internally reset.
> > */
> > #define FLEXCOMM_I2C_CFG_MSTEN (1 << 4)
>
> The FIELD macro already gives you that:
> FIELD(FLEXCOMM_I2C_CFG, MSTEN, startbit, len);
> will define an R_FLEXCOMM_I2C_CFG_MSTEN_MASK (which is
> (1 << startbit) for the 'len == 1' case).
>
> You can also set and read a 1 bit field the same as
> any other, with the FIELD_DP32/FIELD_EX32 macros, so
> you don't often need to directly use the MASK macro:
> s->cfg = FIELD_DP32(s->cfg, CFG, MSTEN, 1);
> and
> if (FIELD_EX32(s->cfg, CFG, MSTEN)) {
> ...
> }
>
> The FIELD() macros are a bit unwieldy sometimes but the
> advantage over ad-hoc #defines is that they're consistent
> for every field in every register.
>
> I agree that providing enums for the possible values for 1-bit
> fields is a bit superfluous.
>
I went ahead and removed those 1-bit enum values and added support to
filter register/fields when generating the code. Also converted the
enums to defines to make these a little bit more compact as I don't
think we have any advantage over the enums?
So with the following invocation:
run_target('svd-flexcomm-i2c', command: svd_gen_header +
[ '-i', rt595, '-o', '@SOURCE_ROOT@/include/hw/arm/svd/flexcomm_i2c.h',
'-p', 'I2C0', '-t', 'FLEXCOMM_I2C',
'--fields', 'CFG TIMEOUT:TOMIN MSTCTL MSTDAT
STAT:MSTPENDING,MSTSTATE INT*:MSTPENDING* SLV*:'])
I am getting the below generated file. Note that the register info is
generated for all registers because this information is used to
initialize the reset values, mask writes appropriately in registers
and trace register access.
Please let me know if this looks good or if there are any other tweaks
I could make.
/*
* Copyright 2016-2023 NXP SPDX-License-Identifier: BSD-3-Clause
*
* Automatically generated by svd-gen-header.py from MIMXRT595S_cm33.xml
*/
#pragma once
#include "hw/register.h"
/* I2C Bus Interface */
#define FLEXCOMM_I2C_REGS_NO (1024)
/* Configuration Register */
REG32(FLEXCOMM_I2C_CFG, 0x800);
/* Master Enable */
FIELD(FLEXCOMM_I2C_CFG, MSTEN, 0, 1);
/* Slave Enable */
FIELD(FLEXCOMM_I2C_CFG, SLVEN, 1, 1);
/* Monitor Enable */
FIELD(FLEXCOMM_I2C_CFG, MONEN, 2, 1);
/* I2C bus Time-out Enable */
FIELD(FLEXCOMM_I2C_CFG, TIMEOUTEN, 3, 1);
/* Monitor function Clock Stretching */
FIELD(FLEXCOMM_I2C_CFG, MONCLKSTR, 4, 1);
/* High Speed mode Capable enable */
FIELD(FLEXCOMM_I2C_CFG, HSCAPABLE, 5, 1);
/* Status Register */
REG32(FLEXCOMM_I2C_STAT, 0x804);
/* Master Pending */
FIELD(FLEXCOMM_I2C_STAT, MSTPENDING, 0, 1);
/* Master State code */
FIELD(FLEXCOMM_I2C_STAT, MSTSTATE, 1, 3);
/* Idle. The Master function is available to be used for a new transaction. */
#define FLEXCOMM_I2C_STAT_MSTSTATE_IDLE 0
/*
* Receive ready. Received data is available (in Master Receiver mode). Address
* plus Read was previously sent and Acknowledged by a slave.
*/
#define FLEXCOMM_I2C_STAT_MSTSTATE_RECEIVE_READY 1
/*
* Transmit ready. Data can be transmitted (in Master Transmitter mode).
* Address plus Write was previously sent and Acknowledged by a slave.
*/
#define FLEXCOMM_I2C_STAT_MSTSTATE_TRANSMIT_READY 2
/* NACK Address. Slave NACKed address. */
#define FLEXCOMM_I2C_STAT_MSTSTATE_NACK_ADDRESS 3
/* NACK Data. Slave NACKed transmitted data. */
#define FLEXCOMM_I2C_STAT_MSTSTATE_NACK_DATA 4
/* Interrupt Enable Set Register */
REG32(FLEXCOMM_I2C_INTENSET, 0x808);
/* Master Pending interrupt Enable */
FIELD(FLEXCOMM_I2C_INTENSET, MSTPENDINGEN, 0, 1);
/* Interrupt Enable Clear Register */
REG32(FLEXCOMM_I2C_INTENCLR, 0x80C);
/* Master Pending interrupt clear */
FIELD(FLEXCOMM_I2C_INTENCLR, MSTPENDINGCLR, 0, 1);
/* Time-out Register */
REG32(FLEXCOMM_I2C_TIMEOUT, 0x810);
/* Time-out time value, the bottom 4 bits */
FIELD(FLEXCOMM_I2C_TIMEOUT, TOMIN, 0, 4);
/* Interrupt Status Register */
REG32(FLEXCOMM_I2C_INTSTAT, 0x818);
/* Master Pending */
FIELD(FLEXCOMM_I2C_INTSTAT, MSTPENDING, 0, 1);
/* Master Control Register */
REG32(FLEXCOMM_I2C_MSTCTL, 0x820);
/* Master Continue(write-only) */
FIELD(FLEXCOMM_I2C_MSTCTL, MSTCONTINUE, 0, 1);
/* Master Start control(write-only) */
FIELD(FLEXCOMM_I2C_MSTCTL, MSTSTART, 1, 1);
/* Master Stop control(write-only) */
FIELD(FLEXCOMM_I2C_MSTCTL, MSTSTOP, 2, 1);
/* Master DMA enable */
FIELD(FLEXCOMM_I2C_MSTCTL, MSTDMA, 3, 1);
/* Master Data Register */
REG32(FLEXCOMM_I2C_MSTDAT, 0x828);
/* Master function data register */
FIELD(FLEXCOMM_I2C_MSTDAT, DATA, 0, 8);
/* Slave Control Register */
REG32(FLEXCOMM_I2C_SLVCTL, 0x840);
/* Slave Data Register */
REG32(FLEXCOMM_I2C_SLVDAT, 0x844);
/* Slave Address Register */
REG32(FLEXCOMM_I2C_SLVADR0, 0x848);
/* Slave Address Register */
REG32(FLEXCOMM_I2C_SLVADR1, 0x84C);
/* Slave Address Register */
REG32(FLEXCOMM_I2C_SLVADR2, 0x850);
/* Slave Address Register */
REG32(FLEXCOMM_I2C_SLVADR3, 0x854);
/* Slave Qualification for Address 0 Register */
REG32(FLEXCOMM_I2C_SLVQUAL0, 0x858);
#define FLEXCOMM_I2C_REGISTER_ACCESS_INFO_ARRAY(_name) \
struct RegisterAccessInfo _name[FLEXCOMM_I2C_REGS_NO] = { \
[0 ... FLEXCOMM_I2C_REGS_NO - 1] = { \
.name = "", \
.addr = -1, \
}, \
[0x200] = { \
.name = "CFG", \
.addr = 0x800, \
.ro = 0xFFFFFFC0, \
.reset = 0x0, \
}, \
[0x201] = { \
.name = "STAT", \
.addr = 0x804, \
.ro = 0xFCF57FAF, \
.reset = 0x801, \
}, \
[0x202] = { \
.name = "INTENSET", \
.addr = 0x808, \
.ro = 0xFCF476AE, \
.reset = 0x0, \
}, \
[0x203] = { \
.name = "INTENCLR", \
.addr = 0x80C, \
.ro = 0xFCF476AE, \
.reset = 0x0, \
}, \
[0x204] = { \
.name = "TIMEOUT", \
.addr = 0x810, \
.ro = 0xFFFF0000, \
.reset = 0xFFFF, \
}, \
[0x205] = { \
.name = "CLKDIV", \
.addr = 0x814, \
.ro = 0xFFFF0000, \
.reset = 0x0, \
}, \
[0x206] = { \
.name = "INTSTAT", \
.addr = 0x818, \
.ro = 0xFFFFFFFF, \
.reset = 0x801, \
}, \
[0x208] = { \
.name = "MSTCTL", \
.addr = 0x820, \
.ro = 0xFFFFFFF0, \
.reset = 0x0, \
}, \
[0x209] = { \
.name = "MSTTIME", \
.addr = 0x824, \
.ro = 0xFFFFFF88, \
.reset = 0x77, \
}, \
[0x20A] = { \
.name = "MSTDAT", \
.addr = 0x828, \
.ro = 0xFFFFFF00, \
.reset = 0x0, \
}, \
[0x210] = { \
.name = "SLVCTL", \
.addr = 0x840, \
.ro = 0xFFFFFCF4, \
.reset = 0x0, \
}, \
[0x211] = { \
.name = "SLVDAT", \
.addr = 0x844, \
.ro = 0xFFFFFF00, \
.reset = 0x0, \
}, \
[0x212] = { \
.name = "SLVADR0", \
.addr = 0x848, \
.ro = 0xFFFF7F00, \
.reset = 0x1, \
}, \
[0x213] = { \
.name = "SLVADR1", \
.addr = 0x84C, \
.ro = 0xFFFF7F00, \
.reset = 0x1, \
}, \
[0x214] = { \
.name = "SLVADR2", \
.addr = 0x850, \
.ro = 0xFFFF7F00, \
.reset = 0x1, \
}, \
[0x215] = { \
.name = "SLVADR3", \
.addr = 0x854, \
.ro = 0xFFFF7F00, \
.reset = 0x1, \
}, \
[0x216] = { \
.name = "SLVQUAL0", \
.addr = 0x858, \
.ro = 0xFFFFFF00, \
.reset = 0x0, \
}, \
[0x220] = { \
.name = "MONRXDAT", \
.addr = 0x880, \
.ro = 0xFFFFFFFF, \
.reset = 0x0, \
}, \
[0x3FF] = { \
.name = "ID", \
.addr = 0xFFC, \
.ro = 0xFFFFFFFF, \
.reset = 0xE0301300, \
}, \
}
