Chris, This approach was also used in bsps/arm/xilinx-zynq/include/bsp/i2c.h. I kept it specifically for consistency; I assumed it was the standard approach, but your response makes me think it's not. Is there a case to be made for breaking the register functions in both i2c.h files out into their own c files? Thanks, Stephen
> -----Original Message----- > From: Chris Johns <chr...@rtems.org> > Sent: Monday, August 23, 2021 9:38 PM > To: Stephen Clark <stephen.cl...@oarcorp.com>; devel@rtems.org > Subject: Re: [PATCH rtems v1 2/2] bsps/zynqmp: Added I2C support for ZynqMP > > On 24/8/21 8:24 am, Stephen Clark wrote: > > Added I2C drivers for ZynqMP and updated build system accordingly. > > --- > > bsps/aarch64/xilinx-zynqmp/include/bsp.h | 4 ++ > > bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h | 63 > > +++++++++++++++++++ bsps/aarch64/xilinx-zynqmp/include/bsp/irq.h | 2 + > > bsps/aarch64/xilinx-zynqmp/start/bspstart.c | 10 +++ > > spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml | 4 ++ > > .../bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml | 2 + > > .../aarch64/xilinx-zynqmp/objcadencei2c.yml | 19 ++++++ > > .../bsps/aarch64/xilinx-zynqmp/optclki2c0.yml | 19 ++++++ > > .../bsps/aarch64/xilinx-zynqmp/optclki2c1.yml | 19 ++++++ > > 9 files changed, 142 insertions(+) > > create mode 100644 bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h > > create mode 100644 > > spec/build/bsps/aarch64/xilinx-zynqmp/objcadencei2c.yml > > create mode 100644 > > spec/build/bsps/aarch64/xilinx-zynqmp/optclki2c0.yml > > create mode 100644 > > spec/build/bsps/aarch64/xilinx-zynqmp/optclki2c1.yml > > > > diff --git a/bsps/aarch64/xilinx-zynqmp/include/bsp.h > > b/bsps/aarch64/xilinx-zynqmp/include/bsp.h > > index 83f2e2f4e4..6d49b9ad2a 100644 > > --- a/bsps/aarch64/xilinx-zynqmp/include/bsp.h > > +++ b/bsps/aarch64/xilinx-zynqmp/include/bsp.h > > @@ -70,6 +70,10 @@ BSP_START_TEXT_SECTION void > > zynqmp_setup_mmu_and_cache(void); > > > > void zynqmp_debug_console_flush(void); > > > > +uint32_t zynqmp_clock_i2c0(void); > > + > > +uint32_t zynqmp_clock_i2c1(void); > > + > > #ifdef __cplusplus > > } > > #endif /* __cplusplus */ > > diff --git a/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h > > b/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h > > new file mode 100644 > > index 0000000000..e09747d414 > > --- /dev/null > > +++ b/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h > > @@ -0,0 +1,63 @@ > > +/* > > + * SPDX-License-Identifier: BSD-2-Clause > > + * > > + * Copyright (C) 2021 On-Line Applications Research (OAR) > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * 1. Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * 2. Redistributions in binary form must reproduce the above copyright > > + * notice, this list of conditions and the following disclaimer in the > > + * documentation and/or other materials provided with the distribution. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS "AS IS" > > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > LIMITED > > +TO, THE > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > PARTICULAR > > +PURPOSE > > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > > +CONTRIBUTORS BE > > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, > > +OR > > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, > PROCUREMENT > > +OF > > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > > +BUSINESS > > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, > > +WHETHER IN > > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR > > +OTHERWISE) > > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF > > +ADVISED OF THE > > + * POSSIBILITY OF SUCH DAMAGE. > > + */ > > + > > +#ifndef LIBBSP_ARM_XILINX_ZYNQ_I2C_H > > +#define LIBBSP_ARM_XILINX_ZYNQ_I2C_H > > + > > +#include <dev/i2c/cadence-i2c.h> > > +#include <bsp/irq.h> > > +#include <bsp.h> > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif /* __cplusplus */ > > + > > +static inline int zynqmp_register_i2c_0(void) { > > + return i2c_bus_register_cadence( > > + "/dev/i2c-0", > > + 0x00FF020000, > > + zynqmp_clock_i2c0(), > > + ZYNQMP_IRQ_I2C_0 > > + ); > > +} > > + > > +static inline int zynqmp_register_i2c_1(void) { > > + return i2c_bus_register_cadence( > > + "/dev/i2c-1", > > + 0x00FF030000, > > + zynqmp_clock_i2c1(), > > + ZYNQMP_IRQ_I2C_1 > > + ); > > I know these are currently inlined but I do not know why they are. It is the > only > BSP that does this. Should they be moved to a .c file seem they are being > touched? > > Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel