From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Fri, 12 May 2017 09:26:42 +0200 Subject: [U-Boot] [PATCH v2 2/5] arm: socfpga: Restructure FPGA driver in the preparation to support A10. In-Reply-To: <1494560712.6027.12.camel@intel.com> References: <1494494734-7888-1-git-send-email-tien.fong.chee@intel.com> <1494494734-7888-3-git-send-email-tien.fong.chee@intel.com> <2ba11ed9-426c-96af-d97b-bda7858e1c9e@denx.de> <1494560712.6027.12.camel@intel.com> Message-ID: <71bcb475-991f-cf43-6928-2a79bf06794d@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 05/12/2017 05:45 AM, Chee, Tien Fong wrote: > On Kha, 2017-05-11 at 14:03 +0200, Marek Vasut wrote: >> On 05/11/2017 11:25 AM, tien.fong.chee at intel.com wrote: >>> >>> From: Tien Fong Chee >>> >>> Move FPGA driver which is Gen5 specific code into Gen5 driver file >>> and keeping common FPGA driver intact. All the changes are still >>> keeping >>> in driver/fpga/ and no functional change. Subsequent patch would >>> move >>> header files into include/intel_socfpga and FPGA manager driver >>> from >>> arch/arm into driver/fpga/ . >>> >>> Signed-off-by: Tien Fong Chee >>> --- [...] >>> +int fpgamgr_test_fpga_ready(void); >>> + >>> +#define FPGA_TIMEOUT_CNT 0x1000000 >> Does that need to be in header file ? >> > Yeah, gen5 and Arria10 driver need this macro. Do they share the block of code which uses this macro ? If so, maybe this can be factored out into common code ? Otherwise, wrap this into both drivers, it's not worth to have it here ... >>> +#ifndef __ASSEMBLY__ >>> + >>> +/* Common prototypes */ >>> +int fpgamgr_dclkcnt_set(unsigned long cnt); >>> >>> +#endif /* __ASSEMBLY__ */ >>> #endif /* _FPGA_MANAGER_H_ */ >> [...] >>> >>> diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga_gen5.c >>> similarity index 82% >>> copy from drivers/fpga/socfpga.c >>> copy to drivers/fpga/socfpga_gen5.c >>> index f1b2f2c..269e81e 100644 >>> --- a/drivers/fpga/socfpga.c >>> +++ b/drivers/fpga/socfpga_gen5.c >>> @@ -1,5 +1,5 @@ >>> /* >>> - * Copyright (C) 2012 Altera Corporation >>> + * Copyright (C) 2012-2017 Altera Corporation >> Removal of code doesn't extend copyright IMO. >> > Oh....okay, understood. I will revert back. >>> >>> * All rights reserved. >>> * >>> * SPDX-License-Identifier: BSD-3-Clause >>> @@ -14,9 +14,6 @@ >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> -/* Timeout count */ >>> -#define FPGA_TIMEOUT_CNT 0x1000000 >>> - >>> static struct socfpga_fpga_manager *fpgamgr_regs = >>> (struct socfpga_fpga_manager >>> *)SOCFPGA_FPGAMGRREGS_ADDRESS; >>> static struct socfpga_system_manager *sysmgr_regs = >>> @@ -30,29 +27,6 @@ static void fpgamgr_set_cd_ratio(unsigned long >>> ratio) >>> (ratio & 0x3) << >>> FPGAMGRREGS_CTRL_CDRATIO_LSB); >>> } >>> >>> -static int fpgamgr_dclkcnt_set(unsigned long cnt) >>> -{ >>> - unsigned long i; >>> - >>> - /* Clear any existing done status */ >>> - if (readl(&fpgamgr_regs->dclkstat)) >>> - writel(0x1, &fpgamgr_regs->dclkstat); >>> - >>> - /* Write the dclkcnt */ >>> - writel(cnt, &fpgamgr_regs->dclkcnt); >>> - >>> - /* Wait till the dclkcnt done */ >>> - for (i = 0; i < FPGA_TIMEOUT_CNT; i++) { >>> - if (!readl(&fpgamgr_regs->dclkstat)) >>> - continue; >>> - >>> - writel(0x1, &fpgamgr_regs->dclkstat); >>> - return 0; >>> - } >>> - >>> - return -ETIMEDOUT; >>> -} >>> - >>> /* Start the FPGA programming by initialize the FPGA Manager */ >>> static int fpgamgr_program_init(void) >>> { >>> @@ -143,34 +117,6 @@ static int fpgamgr_program_init(void) >>> return 0; >>> } >>> >>> -/* Write the RBF data to FPGA Manager */ >>> -static void fpgamgr_program_write(const void *rbf_data, unsigned >>> long rbf_size) >>> -{ >>> - uint32_t src = (uint32_t)rbf_data; >>> - uint32_t dst = SOCFPGA_FPGAMGRDATA_ADDRESS; >>> - >>> - /* Number of loops for 32-byte long copying. */ >>> - uint32_t loops32 = rbf_size / 32; >>> - /* Number of loops for 4-byte long copying + trailing >>> bytes */ >>> - uint32_t loops4 = DIV_ROUND_UP(rbf_size % 32, 4); >>> - >>> - asm volatile( >>> - "1: ldmia %0!, {r0-r7}\n" >>> - " stmia %1!, {r0-r7}\n" >>> - " sub %1, #32\n" >>> - " subs %2, #1\n" >>> - " bne 1b\n" >>> - " cmp %3, #0\n" >>> - " beq 3f\n" >>> - "2: ldr %2, [%0], #4\n >>> " >>> - " str %2, [%1]\n" >>> - " subs %3, #1\n" >>> - " bne 2b\n" >>> - "3: nop\n" >>> - : "+r"(src), "+r"(dst), "+r"(loops32), >>> "+r"(loops4) : >>> - : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", >>> "cc"); >>> -} >>> - >>> /* Ensure the FPGA entering config done */ >>> static int fpgamgr_program_poll_cd(void) >>> { >>> @@ -267,7 +213,6 @@ int socfpga_load(Altera_desc *desc, const void >>> *rbf_data, size_t rbf_size) >>> } >>> >>> /* Prior programming the FPGA, all bridges need to be shut >>> off */ >>> - >> Drop this bit >> > Are you means dropping "-"? Ermm... how to drop this as this is > generated from git. You deleted a line ... re-add it ... it's generated by you, not by git. >>> /* Disable all signals from hps peripheral controller to >>> fpga */ >>> writel(0, &sysmgr_regs->fpgaintfgrp_module); >>> >>> -- Best regards, Marek Vasut