Chris,
>> +
>> +#define ZYNQ_SLCR_BASE_ADDR ( 0xF8000000 )
>> +/* NOTE: QEMU gives a value of 0 for the pss_idcode. */
>> +#define ZYNQ_SLCR_PSS_IDCODE_OFF ( 0x530 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_GET( reg ) BSP_FLD32GET( reg, 12,
>> 16 )
>> +#define ZYNQ_SLCR_LVL_SHFTR_EN_OFF ( 0x900 )
>> +#define ZYNQ_SLCR_LVL_SHFTR_EN_DISABLE ( 0 )
>> +#define ZYNQ_SLCR_LVL_SHFTR_EN_PS_TO_PL ( 0xA )
>> +#define ZYNQ_SLCR_LVL_SHFTR_EN_ALL ( 0xF )
>> +#define ZYNQ_SLCR_LOCK_OFF ( 0x4 )
>> +#define ZYNQ_SLCR_LOCK_KEY ( 0x767b )
>> +#define ZYNQ_SLCR_UNLOCK_OFF ( 0x8 )
>> +#define ZYNQ_SLCR_UNLOCK_KEY ( 0xdf0d )
>> +#define ZYNQ_SLCR_FPGA_RST_CTRL_OFF ( 0x240 )
>>
>
> Can I please suggest the offsets for the registers be grouped together and
> then the values, offsets etc for each register grouped together? It makes
> it easier to extend what you have started to define here.
Sure, I agree.
>
>
> +
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z007s ( 0x03 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z012s ( 0x1c )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z014s ( 0x08 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z010 ( 0x02 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z015 ( 0x1b )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z020 ( 0x07 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z030 ( 0x0c )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z035 ( 0x12 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z045 ( 0x11 )
>> +#define ZYNQ_SLCR_PSS_IDCODE_DEVICE_7z100 ( 0x16 )
>> +
>>
>
> Should there be some register access functions provided? For example:
>
> static inline void zynq_slcr_write_32(const uint32_t reg,
> const uint32_t value)
> {
> volatile uint32_t* slcr;
> slcr = (volatile uint32_t*)(ZYNQ_SLCR_BASE_ADDR + reg);
> *slcr = value;
> }
>
> static inline uint32_t zynq_slcr_read_32(const uint32_t reg)
> {
> volatile uint32_t* slcr;
> return *slcr;
> }
>
> I suggest this because working with the Zynq sometime requires you trace
> the register accesses and this approach allows this. A weakness of a mapped
> struct is not being able to do that.
Makes sense, I will add register access functions.
>
>
> +#ifdef __cplusplus
>> +}
>> +#endif /* __cplusplus */
>> +
>> +#endif /* LIBBSP_ARM_XILINX_ZYNQ_SLCR_REGS_H */
>> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/include/zynq-slcr.h
>> b/c/src/lib/libbsp/arm/xilinx-zynq/include/zynq-slcr.h
>> new file mode 100644
>> index 0000000..2303ff5
>> --- /dev/null
>> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/include/zynq-slcr.h
>> @@ -0,0 +1,70 @@
>> +/**
>> + * @file
>> + * @ingroup zynq_slcr
>> + * @brief SLCR support.
>> + */
>> +
>> +/*
>> + *
>> + * Copyright (c) 2017
>> + * NSF Center for High-Performance Reconfigurable Computing (CHREC),
>> + * University of Pittsburgh. All rights reserved.
>> + *
>> + * 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.
>> + *
>> + * The views and conclusions contained in the software and documentation
>> + * are those of the authors and should not be interpreted as representing
>> + * official policies, either expressed or implied, of CHREC.
>> + *
>> + * Author: Patrick Gauvin <gau...@hcs.ufl.edu>
>> + */
>> +
>> +/**
>> + * @defgroup zynq_slcr SLCR Support
>> + * @ingroup arm_zynq
>> + * @brief SLCR Support
>> + */
>> +
>> +#ifndef LIBBSP_ARM_XILINX_ZYNQ_SLCR_H
>> +#define LIBBSP_ARM_XILINX_ZYNQ_SLCR_H
>> +
>> +#include <stdint.h>
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif /* __cplusplus */
>> +
>> +uint32_t zynq_slcr_pss_idcode_get( void );
>> +
>> +void zynq_slcr_fpga_clk_rst(
>> + uint32_t val
>> +);
>> +
>> +void zynq_slcr_level_shifter_enable(
>> + uint32_t val
>> +);
>>
>
> Any chance you could provide info about the calls?
Sure, I'll add Doxygen comments.
>
>
> +
>> +#ifdef __cplusplus
>> +}
>> +#endif /* __cplusplus */
>> +
>> +#endif /* LIBBSP_ARM_XILINX_ZYNQ_SLCR_H */
>> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/preinstall.am
>> b/c/src/lib/libbsp/arm/xilinx-zynq/preinstall.am
>> index a75c344..814095f 100644
>> --- a/c/src/lib/libbsp/arm/xilinx-zynq/preinstall.am
>> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/preinstall.am
>> @@ -150,6 +150,14 @@ $(PROJECT_INCLUDE)/bsp/zynq-uart-regs.h:
>> include/zynq-uart-regs.h $(PROJECT_INCL
>> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/bsp/zynq-uart-regs.h
>> PREINSTALL_FILES += $(PROJECT_INCLUDE)/bsp/zynq-uart-regs.h
>>
>> +$(PROJECT_INCLUDE)/bsp/zynq-slcr.h: include/zynq-slcr.h
>> $(PROJECT_INCLUDE)/bsp/$(dirstamp)
>> + $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/bsp/zynq-slcr.h
>> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/bsp/zynq-slcr.h
>> +
>> +$(PROJECT_INCLUDE)/bsp/zynq-slcr-regs.h: include/zynq-slcr-regs.h
>> $(PROJECT_INCLUDE)/bsp/$(dirstamp)
>> + $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/bsp/zynq-slcr-regs.h
>> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/bsp/zynq-slcr-regs.h
>> +
>> $(PROJECT_INCLUDE)/libcpu/arm-cp15.h:
>> ../../../libcpu/arm/shared/include/arm-cp15.h
>> $(PROJECT_INCLUDE)/libcpu/$(dirstamp)
>> $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/libcpu/arm-cp15.h
>> PREINSTALL_FILES += $(PROJECT_INCLUDE)/libcpu/arm-cp15.h
>> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/slcr/zynq-slcr.c
>> b/c/src/lib/libbsp/arm/xilinx-zynq/slcr/zynq-slcr.c
>> new file mode 100644
>> index 0000000..8b10722
>> --- /dev/null
>> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/slcr/zynq-slcr.c
>> @@ -0,0 +1,90 @@
>> +/*
>> + * SLCR Support Implementation
>> + *
>> + * At this point, only a few operations related to programming the FPGA
>> are
>> + * supported.
>> + *
>> + * Copyright (c) 2017
>> + * NSF Center for High-Performance Reconfigurable Computing (CHREC),
>> + * University of Pittsburgh. All rights reserved.
>> + *
>> + * 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.
>> + *
>> + * The views and conclusions contained in the software and documentation
>> + * are those of the authors and should not be interpreted as representing
>> + * official policies, either expressed or implied, of CHREC.
>> + *
>> + * Author: Patrick Gauvin <gau...@hcs.ufl.edu>
>> + */
>> +#include <stdint.h>
>> +#include <bsp/zynq-slcr.h>
>> +#include <bsp/zynq-slcr-regs.h>
>> +
>> +static void slcr_unlock( void )
>> +{
>> + volatile uint32_t *unlock;
>> +
>> + unlock = (uint32_t *)( ZYNQ_SLCR_BASE_ADDR + ZYNQ_SLCR_UNLOCK_OFF );
>> + *unlock = ZYNQ_SLCR_UNLOCK_KEY;
>> +}
>>
>
> zynq_slcr_write_32(ZYNQ_SLCR_UNLOCK_OFF, ZYNQ_SLCR_UNLOCK_KEY);
> ?? :)
>
> Doing this lets a simple grep can find all the accesses, plus we do not
> need to remember or check all the accesses are volatile.
>
> +
>> +static void slcr_lock( void )
>> +{
>> + volatile uint32_t *lock;
>> +
>> + lock = (uint32_t *)( ZYNQ_SLCR_BASE_ADDR + ZYNQ_SLCR_LOCK_OFF );
>> + *lock = ZYNQ_SLCR_LOCK_KEY;
>>
>
> +}
>> +
>> +void zynq_slcr_fpga_clk_rst(
>> + uint32_t val
>> +)
>> +{
>> + volatile uint32_t *fpga_rst_ctrl;
>> +
>> + fpga_rst_ctrl =
>> + (uint32_t *)( ZYNQ_SLCR_BASE_ADDR + ZYNQ_SLCR_FPGA_RST_CTRL_OFF );
>> + slcr_unlock();
>> + *fpga_rst_ctrl = 0xf & val;
>> + slcr_lock();
>>
>
> I so not like the reset value being written to like this. I know you need
> to set the lower 4 bits to 1 when loading the fabric via PCAP but there are
> 4 independent resets and I have applications that have individual control
> once loaded. What about a mask be provided and only the bits we are
> interested in are set or cleared depending on the mask?
>
> Note adding mask support will require a lock to be SMP safe.
I see, masking sounds good. Since the lock will need to be initialized
before the SLCR functions are called, should I turn this into a full driver
like device config, or is it acceptable to call an SLCR init function
during BSP startup?
Thank you for the quick feedback,
Patrick
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
